linux-kernel - [RFC][PATCHES] drivers/scsi/sg.c uaccess cleanups/fixes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20191017193659.GA18702@ZenIV.linux.org.uk>
Date:   Thu, 17 Oct 2019 20:36:59 +0100
From:   Al Viro <viro@...iv.linux.org.uk>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     linux-scsi@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: [RFC][PATCHES] drivers/scsi/sg.c uaccess cleanups/fixes

On Wed, Oct 16, 2019 at 09:25:40PM +0100, Al Viro wrote:

> FWIW, callers of __copy_from_user() remaining in the generic code:

> 6) drivers/scsi/sg.c nest: sg_read() ones are memdup_user() in disguise
> (i.e. fold with immediately preceding kmalloc()s).  sg_new_write() -
> fold with access_ok() into copy_from_user() (for both call sites).
> sg_write() - lose access_ok(), use copy_from_user() (both call sites)
> and get_user() (instead of the solitary __get_user() there).

Turns out that there'd been outright redundant access_ok() calls (not
even warranted by __copy_...) *and* several __put_user()/__get_user()
with no checking of return value (access_ok() was there, handling of
unmapped addresses wasn't).  The latter go back at least to 2.1.early...

I've got a series that presumably fixes and cleans the things up
in that area; it didn't get any serious testing (the kernel builds
and boots, smartctl works as well as it used to, but that's not
worth much - all it says is that SG_IO doesn't fail terribly;
I don't have any test setup for really working with /dev/sg*).

IOW, it needs more review and testing - this is _not_ a pull request.
It's in vfs.git#work.sg; individual patches are in followups.
Shortlog/diffstat:
Al Viro (8):
      sg_ioctl(): fix copyout handling
      sg_new_write(): replace access_ok() + __copy_from_user() with copy_from_user()
      sg_write(): __get_user() can fail...
      sg_read(): simplify reading ->pack_id of userland sg_io_hdr_t
      sg_new_write(): don't bother with access_ok
      sg_read(): get rid of access_ok()/__copy_..._user()
      sg_write(): get rid of access_ok()/__copy_from_user()/__get_user()
      SG_IO: get rid of access_ok()

 drivers/scsi/sg.c | 98 ++++++++++++++++++++++++++++++++----------------------------------------------------------------
 1 file changed, 32 insertions(+), 66 deletions(-)