kernel.git/drivers/scsi/sg.c, branch linux-5.5.y

compat: scsi: sg: fix v3 compat read/write interface

2020-02-11T12:37:17Z

commit 78ed001d9e7106171e0ee761cd854137dd731302 upstream. In the v5.4 merge window, a cleanup patch from Al Viro conflicted with my rework of the compat handling for sg.c read(). Linus Torvalds did a correct merge but pointed out that the resulting code is still unsatisfactory. I later noticed that the sg_new_read() function still gets the compat mode wrong, when the 'count' argument is large enough to pass a compat_sg_io_hdr object, but not a nativ sg_io_hdr. To address both of these, move the definition of compat_sg_io_hdr into a scsi/sg.h to make it visible to sg.c and rewrite the logic for reading req_pack_id as well as the size check to a simpler version that gets the expected results. Fixes: c35a5cfb4150 ("scsi: sg: sg_read(): simplify reading ->pack_id of userland sg_io_hdr_t") Fixes: 98aaaec4a150 ("compat_ioctl: reimplement SG_IO handling") Reviewed-by: Ben Hutchings Signed-off-by: Arnd Bergmann Signed-off-by: Greg Kroah-Hartman

Merge tag 'scsi-misc' of git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi

2019-12-02T21:37:02Z

Pull SCSI updates from James Bottomley: "This is mostly update of the usual drivers: aacraid, ufs, zfcp, NCR5380, lpfc, qla2xxx, smartpqi, hisi_sas, target, mpt3sas, pm80xx plus a whole load of minor updates and fixes. The major core changes are Al Viro's reworking of sg's handling of copy to/from user, Ming Lei's removal of the host busy counter to avoid contention in the multiqueue case and Damien Le Moal's fixing of residual tracking across error handling" * tag 'scsi-misc' of git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi: (251 commits) scsi: bnx2fc: timeout calculation invalid for bnx2fc_eh_abort() scsi: target: core: Fix a pr_debug() argument scsi: iscsi: Don't send data to unbound connection scsi: target: iscsi: Wait for all commands to finish before freeing a session scsi: target: core: Release SPC-2 reservations when closing a session scsi: target: core: Document target_cmd_size_check() scsi: bnx2i: fix potential use after free Revert "scsi: qla2xxx: Fix memory leak when sending I/O fails" scsi: NCR5380: Add disconnect_mask module parameter scsi: NCR5380: Unconditionally clear ICR after do_abort() scsi: NCR5380: Call scsi_set_resid() on command completion scsi: scsi_debug: num_tgts must be >= 0 scsi: lpfc: use hdwq assigned cpu for allocation scsi: arcmsr: fix indentation issues scsi: qla4xxx: fix double free bug scsi: pm80xx: Modified the logic to collect fatal dump scsi: pm80xx: Tie the interrupt name to the module instance scsi: pm80xx: Controller fatal error through sysfs scsi: pm80xx: Do not request 12G sas speeds scsi: pm80xx: Cleanup command when a reset times out ...

scsi: sg: sg_ioctl(): get rid of access_ok()

2019-11-06T05:04:03Z

simply not needed there - neither sg_new_read() nor sg_new_write() need it. Link: https://lore.kernel.org/r/20191017193925.25539-8-viro@ZenIV.linux.org.uk Signed-off-by: Al Viro Acked-by: Douglas Gilbert Signed-off-by: Martin K. Petersen

scsi: sg: sg_write(): get rid of access_ok()/__copy_from_user()/__get_user()

2019-11-06T05:04:03Z

Just use plain copy_from_user() and get_user(). Note that while a buf-derived pointer gets stored into ->dxferp, all places that actually use the resulting value feed it either to import_iovec() or to import_single_range(), and both will do validation. Link: https://lore.kernel.org/r/20191017193925.25539-7-viro@ZenIV.linux.org.uk Signed-off-by: Al Viro Acked-by: Douglas Gilbert Signed-off-by: Martin K. Petersen

scsi: sg: sg_read(): get rid of access_ok()/__copy_..._user()

2019-11-06T05:04:03Z

Use copy_..._user() instead, both in sg_read() and in sg_read_oxfer(). And don't open-code memdup_user()... Link: https://lore.kernel.org/r/20191017193925.25539-6-viro@ZenIV.linux.org.uk Signed-off-by: Al Viro Acked-by: Douglas Gilbert Signed-off-by: Martin K. Petersen

scsi: sg: sg_new_write(): don't bother with access_ok

2019-11-06T05:04:03Z

... just use copy_from_user(). We copy only SZ_SG_IO_HDR bytes, so that would, strictly speaking, loosen the check. However, for call chains via ->write() the caller has actually checked the entire range and SG_IO passes exactly SZ_SG_IO_HDR for count. So no visible behaviour changes happen if we check only what we really need for copyin. Link: https://lore.kernel.org/r/20191017193925.25539-5-viro@ZenIV.linux.org.uk Signed-off-by: Al Viro Acked-by: Douglas Gilbert Signed-off-by: Martin K. Petersen

scsi: sg: sg_read(): simplify reading ->pack_id of userland sg_io_hdr_t

2019-11-06T05:04:03Z

We don't need to allocate a temporary buffer and read the entire structure in it, only to fetch a single field and free what we'd allocated. Just use get_user() and be done with it... Link: https://lore.kernel.org/r/20191017193925.25539-4-viro@ZenIV.linux.org.uk Signed-off-by: Al Viro Acked-by: Douglas Gilbert Signed-off-by: Martin K. Petersen

scsi: sg: sg_write(): __get_user() can fail...

2019-11-06T05:04:03Z

Link: https://lore.kernel.org/r/20191017193925.25539-3-viro@ZenIV.linux.org.uk Signed-off-by: Al Viro Acked-by: Douglas Gilbert Signed-off-by: Martin K. Petersen

scsi: sg: sg_new_write(): replace access_ok() + __copy_from_user() with copy_from_user()

2019-11-06T05:04:03Z

Link: https://lore.kernel.org/r/20191017193925.25539-2-viro@ZenIV.linux.org.uk Signed-off-by: Al Viro Acked-by: Douglas Gilbert Signed-off-by: Martin K. Petersen

scsi: sg: sg_ioctl(): fix copyout handling

2019-11-06T05:04:03Z

First of all, __put_user() can fail with access_ok() succeeding. And access_ok() + __copy_to_user() is spelled copy_to_user()... __put_user() *can* fail with access_ok() succeeding... Link: https://lore.kernel.org/r/20191017193925.25539-1-viro@ZenIV.linux.org.uk Signed-off-by: Al Viro Acked-by: Douglas Gilbert Signed-off-by: Martin K. Petersen