On 2019-10-17 9:36 p.m., Al Viro wrote:
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(-)
Al,
I am aware of these and have a 23 part patchset on the linux-scsi list
for review (see https://marc.info/?l=linux-scsi&m=157052102631490&w=2 )
that amongst other things fixes all of these. It also re-adds the
functionality removed from the bsg driver last year. Unfortunately that
review process is going very slowly, so I have no objections if you
apply these now.
It is unlikely that these changes will introduce any bugs (they didn't in
my testing). If you want to do more testing you may find the sg3_utils
package helpful, especially in the testing directory:
https://github.com/hreinecke/sg3_utils
Doug Gilbert
