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


Groups > linux.kernel > #1641006

Re: [git pull] uaccess-related bits of vfs.git

From Al Viro <viro@ZenIV.linux.org.uk>
Newsgroups linux.kernel
Subject Re: [git pull] uaccess-related bits of vfs.git
Date 2017-05-13 22:40 +0200
Message-ID <tGLZw-cm-3@gated-at.bofh.it> (permalink)
References (4 earlier) <tGHVU-5Ze-5@gated-at.bofh.it> <tGIIi-6ye-11@gated-at.bofh.it> <tGIRY-6C1-17@gated-at.bofh.it> <tGJEl-7aE-3@gated-at.bofh.it> <tGKAq-7Ln-7@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Sat, May 13, 2017 at 12:00:10PM -0700, Linus Torvalds wrote:
> From: Linus Torvalds <torvalds@linux-foundation.org>
> Date: Tue, 24 Mar 2015 10:42:18 -0700
> >
> > So I'd suggest we should just do a wholesale replacement of
> > __copy_to/from_user() with the non-underlined cases. Then, we could
> > switch insividual ones back - with reasoning of why they matter, and
> > with pointers to how it does access_ok() two lines before.
> >
> > We should probably even consider looking at __get_user/__put_user().
> > Few of them are actually performance-critical.
> 
> Look at that date. It's over two years ago. In the intervening two
> years, how many of those conversions have happened?

Speaking of killing that kind of crap off: there was a question left from the
last cycle that hadn't been sorted out.

SCTP does this in a couple of places:
        /* Check the user passed a healthy pointer.  */
        if (unlikely(!access_ok(VERIFY_READ, addrs, addrs_size)))
                return -EFAULT;

        /* Alloc space for the address array in kernel memory.  */
        kaddrs = kmalloc(addrs_size, GFP_USER | __GFP_NOWARN);
        if (unlikely(!kaddrs))
                return -ENOMEM;

        if (__copy_from_user(kaddrs, addrs, addrs_size)) {
                kfree(kaddrs);
                return -EFAULT;
        }
instead of memdup_user().  Part of the rationale is pretty weak (access_ok()
as sanity check to prevent user-triggerable attempts to allocate too much -
it still can trivially trigger 2G, so it's not worth much), part is more
interesting.  Namely, that whining into the syslog shouldn't be that easy
to trigger.

That's a valid point and it might apply to memdup_user() callers out there.
Potential variants:
	* add an explicit upper bound on the size and turn that into
memdup_user() (and check that all memdup_user() callers are bounded).
	* have memdup_user() itself pass __GFP_NOWARN.
	* add kvmemdup_user() that would use kvmalloc() (with its callers
expected to use kvfree()); see who else might benefit from conversion.

Preferences?

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


Thread

Re: [git pull] uaccess-related bits of vfs.git Linus Torvalds <torvalds@linux-foundation.org> - 2017-05-13 03:10 +0200
  Re: [git pull] uaccess-related bits of vfs.git Al Viro <viro@ZenIV.linux.org.uk> - 2017-05-13 09:00 +0200
    Re: [git pull] uaccess-related bits of vfs.git Adam Borowski <kilobyte@angband.pl> - 2017-05-13 14:10 +0200
      Re: [git pull] uaccess-related bits of vfs.git Brian Gerst <brgerst@gmail.com> - 2017-05-13 15:50 +0200
      Re: [git pull] uaccess-related bits of vfs.git Al Viro <viro@ZenIV.linux.org.uk> - 2017-05-13 18:50 +0200
    Re: [git pull] uaccess-related bits of vfs.git Linus Torvalds <torvalds@linux-foundation.org> - 2017-05-13 18:20 +0200
    Re: [git pull] uaccess-related bits of vfs.git Linus Torvalds <torvalds@linux-foundation.org> - 2017-05-13 18:20 +0200
      Re: [git pull] uaccess-related bits of vfs.git Al Viro <viro@ZenIV.linux.org.uk> - 2017-05-13 19:10 +0200
        Re: [git pull] uaccess-related bits of vfs.git Linus Torvalds <torvalds@linux-foundation.org> - 2017-05-13 19:20 +0200
          Re: [git pull] uaccess-related bits of vfs.git Al Viro <viro@ZenIV.linux.org.uk> - 2017-05-13 20:10 +0200
            Re: [git pull] uaccess-related bits of vfs.git Al Viro <viro@ZenIV.linux.org.uk> - 2017-05-13 20:30 +0200
              Re: [git pull] uaccess-related bits of vfs.git Al Viro <viro@ZenIV.linux.org.uk> - 2017-05-13 21:20 +0200
                Re: [git pull] uaccess-related bits of vfs.git Al Viro <viro@ZenIV.linux.org.uk> - 2017-05-13 21:40 +0200
            Re: [git pull] uaccess-related bits of vfs.git Linus Torvalds <torvalds@linux-foundation.org> - 2017-05-13 21:10 +0200
              Re: [git pull] uaccess-related bits of vfs.git Al Viro <viro@ZenIV.linux.org.uk> - 2017-05-13 21:20 +0200
              Re: [git pull] uaccess-related bits of vfs.git Al Viro <viro@ZenIV.linux.org.uk> - 2017-05-13 22:00 +0200
                Re: [git pull] uaccess-related bits of vfs.git Al Viro <viro@ZenIV.linux.org.uk> - 2017-05-13 22:10 +0200
                Re: [git pull] uaccess-related bits of vfs.git Geert Uytterhoeven <geert@linux-m68k.org> - 2017-05-13 22:40 +0200
                Re: [git pull] uaccess-related bits of vfs.git Al Viro <viro@ZenIV.linux.org.uk> - 2017-05-13 22:50 +0200
              Re: [git pull] uaccess-related bits of vfs.git Al Viro <viro@ZenIV.linux.org.uk> - 2017-05-13 22:40 +0200
                Re: [git pull] uaccess-related bits of vfs.git Linus Torvalds <torvalds@linux-foundation.org> - 2017-05-13 23:00 +0200
                Re: [git pull] uaccess-related bits of vfs.git Al Viro <viro@ZenIV.linux.org.uk> - 2017-05-13 23:30 +0200
        Re: [git pull] uaccess-related bits of vfs.git Al Viro <viro@ZenIV.linux.org.uk> - 2017-05-13 19:20 +0200
      Re: [git pull] uaccess-related bits of vfs.git Ingo Molnar <mingo@kernel.org> - 2017-05-14 20:20 +0200
        Re: [git pull] uaccess-related bits of vfs.git Al Viro <viro@ZenIV.linux.org.uk> - 2017-05-14 21:00 +0200

csiph-web