Messages in this thread | | | Subject | Re: [PATCH] scsi: target: pscsi: Fix possible null-pointer dereference in pscsi_complete_cmd() | From | Bodo Stroesser <> | Date | Mon, 9 Aug 2021 12:36:03 +0200 |
| |
On 07.08.21 15:46, Tuo Li wrote: > The return value of transport_kmap_data_sg() is assigned to the variable > buf: > buf = transport_kmap_data_sg(cmd); > > And then it is checked: > if (!buf) { > > This indicates that buf can be NULL. However, it is dereferenced in the > following statements: > if (!(buf[3] & 0x80)) > buf[3] |= 0x80; > if (!(buf[2] & 0x80)) > buf[2] |= 0x80; > > To fix these possible null-pointer dereferences, dereference buf only when > it is not NULL. > > Reported-by: TOTE Robot <oslab@tsinghua.edu.cn> > Signed-off-by: Tuo Li <islituo@gmail.com> > --- > drivers/target/target_core_pscsi.c | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/drivers/target/target_core_pscsi.c b/drivers/target/target_core_pscsi.c > index 2629d2ef3970..560815729182 100644 > --- a/drivers/target/target_core_pscsi.c > +++ b/drivers/target/target_core_pscsi.c > @@ -620,14 +620,14 @@ static void pscsi_complete_cmd(struct se_cmd *cmd, u8 scsi_status, > buf = transport_kmap_data_sg(cmd); > if (!buf) { > ; /* XXX: TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE */ > - } > - > - if (cdb[0] == MODE_SENSE_10) { > - if (!(buf[3] & 0x80)) > - buf[3] |= 0x80; > } else { > - if (!(buf[2] & 0x80)) > - buf[2] |= 0x80; > + if (cdb[0] == MODE_SENSE_10) { > + if (!(buf[3] & 0x80)) > + buf[3] |= 0x80; > + } else { > + if (!(buf[2] & 0x80)) > + buf[2] |= 0x80; > + } > } > > transport_kunmap_data_sg(cmd); >
I'm wondering whether we should better put the transport_kunmap_data_sg into the else-branch of the if (!buf)? AFAICS, calling it after transport_kmap_data_sg failed does not cause problems, but I feel it would be cleaner.
Otherwise it looks good to me.
| |