* [PATCH -next] [SCSI] Fix NULL pointer dereference in handling for passthrough commands
@ 2021-09-04 6:45 Laibin Qiu
2021-09-05 4:49 ` Bart Van Assche
2021-09-07 0:40 ` Bart Van Assche
0 siblings, 2 replies; 4+ messages in thread
From: Laibin Qiu @ 2021-09-04 6:45 UTC (permalink / raw)
To: jejb, martin.petersen, hare; +Cc: linux-scsi, linux-kernel, qiulaibin
In passthrough path. If the command size for ioctl request from userspace
is 0. The original process will get cmd_len from cmd->cmnd, but It has
not been assigned at this time. So it will trigger a NULL pointer BUG.
------------[ cut here ]------------
BUG: kernel NULL pointer dereference, address: 0000000000000000
PF: supervisor read access in kernel mode
PF: error_code(0x0000) - not-present page
RIP: 0010:scsi_queue_rq+0xcb2/0x12b0
Call Trace:
blk_mq_dispatch_rq_list+0x541/0xe90
__blk_mq_sched_dispatch_requests+0x1fe/0x2b0
blk_mq_sched_dispatch_requests+0xbf/0x130
__blk_mq_run_hw_queue+0x15b/0x230
__blk_mq_delay_run_hw_queue+0x18f/0x320
blk_mq_run_hw_queue+0x252/0x280
blk_mq_sched_insert_request+0x228/0x260
blk_execute_rq+0x111/0x160
sg_io+0x51a/0x740
scsi_cmd_ioctl+0x533/0x910
scsi_cmd_blk_ioctl+0xa1/0xb0
cdrom_ioctl+0x3f/0x2510
sr_block_ioctl+0x142/0x180
blkdev_ioctl+0x398/0x450
block_ioctl+0x6d/0x80
__se_sys_ioctl+0xd1/0x140
__x64_sys_ioctl+0x3f/0x50
do_syscall_64+0x37/0x50
entry_SYSCALL_64_after_hwframe+0x44/0xa9
We can trigger front BUG by ioctl blow.
------------[ cut here ]------------
sg_io_hdr_t *addr;
addr = malloc(sizeof(sg_io_hdr_t));
memset(addr, 0, sizeof(sg_io_hdr_t));
addr->interface_id = 'S';
fd = open(/dev/sr0, O_RDONLY); // open a CD_ROM dev
ioctl(fd, SG_IO, addr); // all zero sg_io_hdr_t will trigger this bug
Fixes: 2ceda20f0a99a ("scsi: core: Move command size detection out of
the fast path")
Signed-off-by: Laibin Qiu <qiulaibin@huawei.com>
---
drivers/scsi/scsi_lib.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 572673873ddf..53b47a9103d3 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1174,9 +1174,9 @@ static blk_status_t scsi_setup_scsi_cmnd(struct scsi_device *sdev,
}
cmd->cmd_len = scsi_req(req)->cmd_len;
+ cmd->cmnd = scsi_req(req)->cmd;
if (cmd->cmd_len == 0)
cmd->cmd_len = scsi_command_size(cmd->cmnd);
- cmd->cmnd = scsi_req(req)->cmd;
cmd->transfersize = blk_rq_bytes(req);
cmd->allowed = scsi_req(req)->retries;
return BLK_STS_OK;
--
2.22.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH -next] [SCSI] Fix NULL pointer dereference in handling for passthrough commands
2021-09-04 6:45 [PATCH -next] [SCSI] Fix NULL pointer dereference in handling for passthrough commands Laibin Qiu
@ 2021-09-05 4:49 ` Bart Van Assche
2021-09-07 0:40 ` Bart Van Assche
1 sibling, 0 replies; 4+ messages in thread
From: Bart Van Assche @ 2021-09-05 4:49 UTC (permalink / raw)
To: Laibin Qiu, jejb, martin.petersen, hare; +Cc: linux-scsi, linux-kernel
On 9/3/21 23:45, Laibin Qiu wrote:
> In passthrough path. If the command size for ioctl request from userspace
> is 0. The original process will get cmd_len from cmd->cmnd, but It has
> not been assigned at this time. So it will trigger a NULL pointer BUG.
A Friday night in the middle of the merge window is not the best time to
post a kernel patch. Anyway:
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH -next] [SCSI] Fix NULL pointer dereference in handling for passthrough commands
2021-09-04 6:45 [PATCH -next] [SCSI] Fix NULL pointer dereference in handling for passthrough commands Laibin Qiu
2021-09-05 4:49 ` Bart Van Assche
@ 2021-09-07 0:40 ` Bart Van Assche
2021-09-07 6:54 ` Christoph Hellwig
1 sibling, 1 reply; 4+ messages in thread
From: Bart Van Assche @ 2021-09-07 0:40 UTC (permalink / raw)
To: Laibin Qiu, jejb, martin.petersen, hare; +Cc: linux-scsi, Christoph Hellwig
On 9/3/21 23:45, Laibin Qiu wrote:
> cmd->cmd_len = scsi_req(req)->cmd_len;
> + cmd->cmnd = scsi_req(req)->cmd;
> if (cmd->cmd_len == 0)
> cmd->cmd_len = scsi_command_size(cmd->cmnd);
> - cmd->cmnd = scsi_req(req)->cmd;
> cmd->transfersize = blk_rq_bytes(req);
Thinking further about this: is there any code left that depends on
scsi_setup_scsi_cmnd() setting cmd->cmd_len? Can the cmd->cmd_len
assignment be removed from scsi_setup_scsi_cmnd()?
Thanks,
Bart.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH -next] [SCSI] Fix NULL pointer dereference in handling for passthrough commands
2021-09-07 0:40 ` Bart Van Assche
@ 2021-09-07 6:54 ` Christoph Hellwig
0 siblings, 0 replies; 4+ messages in thread
From: Christoph Hellwig @ 2021-09-07 6:54 UTC (permalink / raw)
To: Bart Van Assche
Cc: Laibin Qiu, jejb, martin.petersen, hare, linux-scsi, Christoph Hellwig
On Mon, Sep 06, 2021 at 05:40:35PM -0700, Bart Van Assche wrote:
> On 9/3/21 23:45, Laibin Qiu wrote:
>> cmd->cmd_len = scsi_req(req)->cmd_len;
>> + cmd->cmnd = scsi_req(req)->cmd;
>> if (cmd->cmd_len == 0)
>> cmd->cmd_len = scsi_command_size(cmd->cmnd);
>> - cmd->cmnd = scsi_req(req)->cmd;
>> cmd->transfersize = blk_rq_bytes(req);
>
> Thinking further about this: is there any code left that depends on
> scsi_setup_scsi_cmnd() setting cmd->cmd_len? Can the cmd->cmd_len
> assignment be removed from scsi_setup_scsi_cmnd()?
cmd_len should never be 0 now, so I think we can remove it.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-09-07 6:54 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-04 6:45 [PATCH -next] [SCSI] Fix NULL pointer dereference in handling for passthrough commands Laibin Qiu
2021-09-05 4:49 ` Bart Van Assche
2021-09-07 0:40 ` Bart Van Assche
2021-09-07 6:54 ` Christoph Hellwig
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.