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@xxxxxxxxxxxxxxx> Signed-off-by: Tuo Li <islituo@xxxxxxxxx> --- 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.
