| Message ID | 20240605213428.4040-1-mish.uxin2012@yandex.ru |
|---|---|
| State | New |
| Headers | show |
| Series | [v3,5.10/5.15] ata: libata-scsi: check cdb length for VARIABLE_LENGTH_CMD commands | expand |
On 6/6/24 6:34 AM, Mikhail Ukhin wrote: > No upstream commit exists for this patch. > > Fuzzing of 5.10 stable branch reports a slab-out-of-bounds error in > ata_scsi_pass_thru. > > The error is fixed in 5.18 by commit ce70fd9a551a ("scsi: core: Remove the > cmd field from struct scsi_request") upstream. > Backporting this commit would require significant changes to the code so > it is bettter to use a simple fix for that particular error. > > The problem is that the length of the received SCSI command is not > validated if scsi_op == VARIABLE_LENGTH_CMD. It can lead to out-of-bounds > reading if the user sends a request with SCSI command of length less than > 32. > > Found by Linux Verification Center (linuxtesting.org) with Syzkaller. > > Signed-off-by: Artem Sadovnikov <ancowi69@gmail.com> > Signed-off-by: Mikhail Ivanov <iwanov-23@bk.ru> > Signed-off-by: Mikhail Ukhin <mish.uxin2012@yandex.ru> > --- > v2: The new addresses were added and the text was updated. > v3: Checking has been moved to the function ata_scsi_var_len_cdb_xlat at > the request of Damien Le Moal > drivers/ata/libata-scsi.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c > index dfa090ccd21c..38488bd813d1 100644 > --- a/drivers/ata/libata-scsi.c > +++ b/drivers/ata/libata-scsi.c > @@ -3948,7 +3948,11 @@ static unsigned int ata_scsi_var_len_cdb_xlat(struct ata_queued_cmd *qc) > struct scsi_cmnd *scmd = qc->scsicmd; > const u8 *cdb = scmd->cmnd; > const u16 sa = get_unaligned_be16(&cdb[8]); > + u8 scsi_op = scmd->cmnd[0]; > > + if (scsi_op == VARIABLE_LENGTH_CMD && scmd->cmd_len < 32) This functions is called only when the opcode is VARIABLE_LENGTH_CMD. So there is no need to check that again. > + return 1; > + > /* > * if service action represents a ata pass-thru(32) command, > * then pass it to ata_scsi_pass_thru handler.
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index dfa090ccd21c..38488bd813d1 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -3948,7 +3948,11 @@ static unsigned int ata_scsi_var_len_cdb_xlat(struct ata_queued_cmd *qc) struct scsi_cmnd *scmd = qc->scsicmd; const u8 *cdb = scmd->cmnd; const u16 sa = get_unaligned_be16(&cdb[8]); + u8 scsi_op = scmd->cmnd[0]; + if (scsi_op == VARIABLE_LENGTH_CMD && scmd->cmd_len < 32) + return 1; + /* * if service action represents a ata pass-thru(32) command, * then pass it to ata_scsi_pass_thru handler.
No upstream commit exists for this patch. Fuzzing of 5.10 stable branch reports a slab-out-of-bounds error in ata_scsi_pass_thru. The error is fixed in 5.18 by commit ce70fd9a551a ("scsi: core: Remove the cmd field from struct scsi_request") upstream. Backporting this commit would require significant changes to the code so it is bettter to use a simple fix for that particular error. The problem is that the length of the received SCSI command is not validated if scsi_op == VARIABLE_LENGTH_CMD. It can lead to out-of-bounds reading if the user sends a request with SCSI command of length less than 32. Found by Linux Verification Center (linuxtesting.org) with Syzkaller. Signed-off-by: Artem Sadovnikov <ancowi69@gmail.com> Signed-off-by: Mikhail Ivanov <iwanov-23@bk.ru> Signed-off-by: Mikhail Ukhin <mish.uxin2012@yandex.ru> --- v2: The new addresses were added and the text was updated. v3: Checking has been moved to the function ata_scsi_var_len_cdb_xlat at the request of Damien Le Moal drivers/ata/libata-scsi.c | 4 ++++ 1 file changed, 4 insertions(+)