* Potential double fetch in sg_scsi_ioctl()
@ 2020-11-23 16:15 Alexander Potapenko
2020-11-23 16:47 ` Jens Axboe
0 siblings, 1 reply; 2+ messages in thread
From: Alexander Potapenko @ 2020-11-23 16:15 UTC (permalink / raw)
To: Jens Axboe, Christoph Hellwig
Cc: Dmitriy Vyukov, Merna Zakaria, linux-scsi, mengxu.gatech
Hi Christof, Jens,
We've found a double-fetch in sg_scsi_ioctl() using a prototype tool
(see the report below).
Turns out that sg_scsi_ioctl() reads the first byte of sic->data
twice: first when getting the opcode
(https://elixir.bootlin.com/linux/latest/source/block/scsi_ioctl.c#L439),
then when reading the command of the size calculated from that opcode
(https://elixir.bootlin.com/linux/latest/source/block/scsi_ioctl.c#L464).
At this point opcode and req->cmd[0] may mismatch.
The opcode is then used to determine rq->timeout and req->retries,
whereas req->cmd[0] is used by the underlying device drivers.
Not sure invalid timeout or retries is a big deal, but since the
command length also depends on the opcode, it is possible to trick the
kernel into using the remnants of the previous command by first
announcing a short command and then changing the opcode to a longer
one.
I've noticed that three years ago Meng Xu has reported the very same
bug already: https://patchwork.kernel.org/project/linux-block/patch/1505834638-37142-1-git-send-email-mengxu.gatech@gmail.com/
Was there any followup to that patch?
Alex
Double fetch report follows:
BUG: multi-read in sg_scsi_ioctl, syscall __x64_sys_ioctl
==================================================================
======= First Address Range Stack =======
df_save_stack+0x33/0x70 lib/df-detection.c:202
add_address+0x2cb/0x370 lib/df-detection.c:48
sg_scsi_ioctl+0x160/0x560 block/scsi_ioctl.c:439
sg_ioctl_common+0xdb6/0x1110 drivers/scsi/sg.c:1109
sg_ioctl+0x49/0xa0 drivers/scsi/sg.c:1163
vfs_ioctl fs/ioctl.c:48 [inline]
__do_sys_ioctl fs/ioctl.c:753 [inline]
__se_sys_ioctl fs/ioctl.c:739 [inline]
__x64_sys_ioctl+0xfc/0x140 fs/ioctl.c:739
do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46
entry_SYSCALL_64_after_hwframe+0x44/0xa9
======= Second Address Range Stack =======
df_save_stack+0x33/0x70 lib/df-detection.c:202
add_address+0x2cb/0x370 lib/df-detection.c:48
_copy_from_user+0x93/0xe0 lib/usercopy.c:17
copy_from_user include/linux/uaccess.h:167 [inline]
sg_scsi_ioctl+0x21a/0x560 block/scsi_ioctl.c:464
sg_ioctl_common+0xdb6/0x1110 drivers/scsi/sg.c:1109
sg_ioctl+0x49/0xa0 drivers/scsi/sg.c:1163
vfs_ioctl fs/ioctl.c:48 [inline]
__do_sys_ioctl fs/ioctl.c:753 [inline]
__se_sys_ioctl fs/ioctl.c:739 [inline]
__x64_sys_ioctl+0xfc/0x140 fs/ioctl.c:739
do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46
entry_SYSCALL_64_after_hwframe+0x44/0xa9
syscall number 16 System Call: __x64_sys_ioctl+0x0/0x140 fs/ioctl.c:800
First 0000000020000188 len 1 Caller sg_ioctl_common+0xdb6/0x1110
drivers/scsi/sg.c:1109
Second 0000000020000188 len 10 Caller copy_from_user
include/linux/uaccess.h:167 [inline]
Second 0000000020000188 len 10 Caller sg_scsi_ioctl+0x21a/0x560
block/scsi_ioctl.c:464
CPU: 0 PID: 6266 Comm: syz-executor.0 Not tainted 5.9.0-rc4+ #55
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.14.0-1 04/01/2014
==================================================================
--
Alexander Potapenko
Software Engineer
Google Germany GmbH
Erika-Mann-Straße, 33
80636 München
Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: Potential double fetch in sg_scsi_ioctl()
2020-11-23 16:15 Potential double fetch in sg_scsi_ioctl() Alexander Potapenko
@ 2020-11-23 16:47 ` Jens Axboe
0 siblings, 0 replies; 2+ messages in thread
From: Jens Axboe @ 2020-11-23 16:47 UTC (permalink / raw)
To: Alexander Potapenko, Christoph Hellwig
Cc: Dmitriy Vyukov, Merna Zakaria, linux-scsi, mengxu.gatech
On 11/23/20 9:15 AM, Alexander Potapenko wrote:
> Hi Christof, Jens,
>
> We've found a double-fetch in sg_scsi_ioctl() using a prototype tool
> (see the report below).
>
> Turns out that sg_scsi_ioctl() reads the first byte of sic->data
> twice: first when getting the opcode
> (https://elixir.bootlin.com/linux/latest/source/block/scsi_ioctl.c#L439),
> then when reading the command of the size calculated from that opcode
> (https://elixir.bootlin.com/linux/latest/source/block/scsi_ioctl.c#L464).
>
> At this point opcode and req->cmd[0] may mismatch.
> The opcode is then used to determine rq->timeout and req->retries,
> whereas req->cmd[0] is used by the underlying device drivers.
> Not sure invalid timeout or retries is a big deal, but since the
> command length also depends on the opcode, it is possible to trick the
> kernel into using the remnants of the previous command by first
> announcing a short command and then changing the opcode to a longer
> one.
>
> I've noticed that three years ago Meng Xu has reported the very same
> bug already: https://patchwork.kernel.org/project/linux-block/patch/1505834638-37142-1-git-send-email-mengxu.gatech@gmail.com/
> Was there any followup to that patch?
Doesn't look like it - Christoph made a suggestion, and then the
original reporter didn't follow up. FWIW, I do agree that just copying
it once is a better idea than copying twice and then copying the opcode
again.
--
Jens Axboe
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-11-23 16:47 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-23 16:15 Potential double fetch in sg_scsi_ioctl() Alexander Potapenko
2020-11-23 16:47 ` Jens Axboe
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.