Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1612438

Re: [RFC][CFT][PATCHSET v1] uaccess unification

From Al Viro <viro@ZenIV.linux.org.uk>
Newsgroups linux.kernel
Subject Re: [RFC][CFT][PATCHSET v1] uaccess unification
Date 2017-03-30 01:20 +0200
Message-ID <tqv2G-5Ex-3@gated-at.bofh.it> (permalink)
References (1 earlier) <tqseu-3C5-27@gated-at.bofh.it> <tqsxQ-3KK-13@gated-at.bofh.it> <tqsxQ-3KK-11@gated-at.bofh.it> <tqt0T-4cP-37@gated-at.bofh.it> <tqtkd-4kM-1@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Wed, Mar 29, 2017 at 02:24:37PM -0700, Linus Torvalds wrote:

> I think one of the biggest problems with our current uaccess.h mess is
> just how illegible the header files are, and the
> INLINE_COPY_{TO,FROM}_USER thing is not helping.
> 
> I think it would be much better if the header file just had
> 
>  extern unsigned long _copy_from_user(void *, const void __user *,
> unsigned long);
> 
> and nothing else. No unnecessary noise.
> 
> The same goes for things like [__]copy_in_user() - why is that thing
> still inlined? If it was a *macro*, it might be useful due to the
> might_fault() thing giving the caller information, but that's not even
> the case here, so we'd actually be much better off without any of that
> inlining stuff. Do it all in lib/usercopy.c, and move the
> might_fault() in there too.

IMO that's a separate series.  For now I would be bloody happy if we got
	* arch-dependent asm fixes out of the way
	* everything consolidated outside of arch/*
	* arch/*/include/uaccess*.h simplified.

As for __copy_in_user()... I'm not sure we want to keep it in the long run -
drivers/gpu/drm/drm_ioc32.c:390:        if (__copy_in_user(buf, argp, offsetof(drm_buf_desc32_t, agp_start))
drivers/gpu/drm/drm_ioc32.c:399:        if (__copy_in_user(argp, buf, offsetof(drm_buf_desc32_t, agp_start))
drivers/gpu/drm/drm_ioc32.c:475:                        if (__copy_in_user(&to[i], &list[i],
drivers/gpu/drm/drm_ioc32.c:536:                        if (__copy_in_user(&list32[i], &list[i],
fs/compat_ioctl.c:753:  if (__copy_in_user(&tdata->read_write, &udata->read_write, 2 * sizeof(u8)))
fs/compat_ioctl.c:755:  if (__copy_in_user(&tdata->size, &udata->size, 2 * sizeof(u32)))

are all callers out there.  And looking at those callers...  fs/compat_ioctl.c
ones are ridiculous - they translate
struct i2c_smbus_ioctl_data {
        __u8 read_write;
        __u8 command;
        __u32 size;
        union i2c_smbus_data __user *data;
};
into
struct i2c_smbus_ioctl_data32 {
        u8 read_write;
        u8 command;
        u32 size;
        compat_caddr_t data; /* union i2c_smbus_data *data */
};
by doing
	* 2 byte copy (read_write + command -> read_write + command)
	* 8 byte copy (size + data -> size + half of data; WTF 8 and not 4?)
	* 4 byte load (data)
	* 8 byte store (data)
That gem went into the tree in 2003, apparently as a quick hack from
benh, and never had been touched since then.  IMO inlining is very far
down the list of, er, deficiencies there.  If anything, it would be
better off with a single copy_from_user() into a local union, followed by
something like foo.native.data = compat_ptr(foo.compat.data) and
copy_to_user() into tdata.  And that's assuming we won't be better off
with proper ->compat_ioctl() for that sucker - AFAICS, there's a bunch
of I2C_... stuff understood by fs/compat_ioctl.c, all for the sake of
one driver.  I'll look into that tonight...

As for the drm ones, I don't see any reasons for them not to be copy_in_user().
If any of that is the hot path (the last two are in loops), we have worse
problems with STAC/CLAC anyway.

So I'm not sure if __copy_in_user() shouldn't just die.  copy_in_user()
might be a good candidate for move to lib/usercopy.c; I'm somewhat worried
about sparc64, though.  access_ok() is a no-op there, so on the builds
without lockdep where might_fault() is a no-op we get pointless extra
jump for no good reason.  I would like to see comments from davem on that
one...

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[RFC][CFT][PATCHSET v1] uaccess unification Al Viro <viro@ZenIV.linux.org.uk> - 2017-03-29 08:00 +0200
  Re: [RFC][CFT][PATCHSET v1] uaccess unification Vineet Gupta <Vineet.Gupta1@synopsys.com> - 2017-03-29 22:20 +0200
    Re: [RFC][CFT][PATCHSET v1] uaccess unification Linus Torvalds <torvalds@linux-foundation.org> - 2017-03-29 22:40 +0200
      Re: [RFC][CFT][PATCHSET v1] uaccess unification Al Viro <viro@ZenIV.linux.org.uk> - 2017-03-29 23:10 +0200
        Re: [RFC][CFT][PATCHSET v1] uaccess unification Linus Torvalds <torvalds@linux-foundation.org> - 2017-03-29 23:30 +0200
          Re: [RFC][CFT][PATCHSET v1] uaccess unification Al Viro <viro@ZenIV.linux.org.uk> - 2017-03-30 01:20 +0200
            Re: [RFC][CFT][PATCHSET v1] uaccess unification Linus Torvalds <torvalds@linux-foundation.org> - 2017-03-30 01:50 +0200
              Re: [RFC][CFT][PATCHSET v1] uaccess unification Al Viro <viro@ZenIV.linux.org.uk> - 2017-03-30 17:40 +0200
    Re: [RFC][CFT][PATCHSET v1] uaccess unification Al Viro <viro@ZenIV.linux.org.uk> - 2017-03-29 22:40 +0200
      Re: [RFC][CFT][PATCHSET v1] uaccess unification Vineet Gupta <Vineet.Gupta1@synopsys.com> - 2017-03-29 23:20 +0200
        Re: [RFC][CFT][PATCHSET v1] uaccess unification Al Viro <viro@ZenIV.linux.org.uk> - 2017-03-30 01:50 +0200
          Re: [RFC][CFT][PATCHSET v1] uaccess unification Vineet Gupta <Vineet.Gupta1@synopsys.com> - 2017-03-30 02:10 +0200
            Re: [RFC][CFT][PATCHSET v1] uaccess unification Linus Torvalds <torvalds@linux-foundation.org> - 2017-03-30 02:40 +0200
              Re: [RFC][CFT][PATCHSET v1] uaccess unification Al Viro <viro@ZenIV.linux.org.uk> - 2017-03-30 03:20 +0200
              Re: [RFC][CFT][PATCHSET v1] uaccess unification Vineet Gupta <Vineet.Gupta1@synopsys.com> - 2017-03-30 22:50 +0200
                Re: [RFC][CFT][PATCHSET v1] uaccess unification Linus Torvalds <torvalds@linux-foundation.org> - 2017-03-30 23:10 +0200
                Re: [RFC][CFT][PATCHSET v1] uaccess unification Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-31 01:30 +0200
  Re: [RFC][CFT][PATCHSET v1] uaccess unification Martin Schwidefsky <schwidefsky@de.ibm.com> - 2017-03-30 14:40 +0200
    Re: [RFC][CFT][PATCHSET v1] uaccess unification Al Viro <viro@ZenIV.linux.org.uk> - 2017-03-30 16:50 +0200
  Re: [RFC][CFT][PATCHSET v1] uaccess unification Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-30 18:30 +0200
    Re: [RFC][CFT][PATCHSET v1] uaccess unification Al Viro <viro@ZenIV.linux.org.uk> - 2017-03-30 18:50 +0200
      Re: [RFC][CFT][PATCHSET v1] uaccess unification Linus Torvalds <torvalds@linux-foundation.org> - 2017-03-30 19:20 +0200
        Re: [RFC][CFT][PATCHSET v1] uaccess unification Al Viro <viro@ZenIV.linux.org.uk> - 2017-03-30 20:50 +0200
          Re: [RFC][CFT][PATCHSET v1] uaccess unification Linus Torvalds <torvalds@linux-foundation.org> - 2017-03-30 21:00 +0200
          Re: [RFC][CFT][PATCHSET v1] uaccess unification Al Viro <viro@ZenIV.linux.org.uk> - 2017-03-30 21:00 +0200
            Re: [RFC][CFT][PATCHSET v1] uaccess unification Linus Torvalds <torvalds@linux-foundation.org> - 2017-03-30 21:00 +0200
              Re: [RFC][CFT][PATCHSET v1] uaccess unification Al Viro <viro@ZenIV.linux.org.uk> - 2017-03-30 21:20 +0200
                Re: [RFC][CFT][PATCHSET v1] uaccess unification Linus Torvalds <torvalds@linux-foundation.org> - 2017-03-30 21:30 +0200
                Re: [RFC][CFT][PATCHSET v1] uaccess unification Al Viro <viro@ZenIV.linux.org.uk> - 2017-03-30 23:20 +0200
  Re: [RFC][CFT][PATCHSET v1] uaccess unification Kees Cook <keescook@chromium.org> - 2017-03-31 02:30 +0200
    Re: [RFC][CFT][PATCHSET v1] uaccess unification James Hogan <james.hogan@imgtec.com> - 2017-03-31 15:40 +0200
  Re: [RFC][CFT][PATCHSET v1] uaccess unification James Morse <james.morse@arm.com> - 2017-04-03 18:30 +0200
  Re: [RFC][CFT][PATCHSET v1] uaccess unification Max Filippov <jcmvbkbc@gmail.com> - 2017-04-04 22:30 +0200
    Re: [RFC][CFT][PATCHSET v1] uaccess unification Al Viro <viro@ZenIV.linux.org.uk> - 2017-04-04 23:00 +0200
  ia64 exceptions (Re: [RFC][CFT][PATCHSET v1] uaccess unification) Al Viro <viro@ZenIV.linux.org.uk> - 2017-04-05 07:10 +0200
    Re: ia64 exceptions (Re: [RFC][CFT][PATCHSET v1] uaccess unification) Al Viro <viro@ZenIV.linux.org.uk> - 2017-04-05 10:10 +0200
      Re: ia64 exceptions (Re: [RFC][CFT][PATCHSET v1] uaccess unification) Tony Luck <tony.luck@gmail.com> - 2017-04-05 20:50 +0200
        Re: ia64 exceptions (Re: [RFC][CFT][PATCHSET v1] uaccess unification) Al Viro <viro@ZenIV.linux.org.uk> - 2017-04-05 22:40 +0200
  [RFC][CFT][PATCHSET v2] uaccess unification Al Viro <viro@ZenIV.linux.org.uk> - 2017-04-07 02:30 +0200
    Re: [RFC][CFT][PATCHSET v2] uaccess unification Al Viro <viro@ZenIV.linux.org.uk> - 2017-04-07 02:40 +0200

csiph-web