On Fri, Oct 30, 2020 at 6:59 AM Viswas G <Viswas.G@xxxxxxxxxxxxxxxxx> wrote:
>
> From: yuuzheng <yuuzheng@xxxxxxxxxx>
>
> The use-after-free or null-pointer error occurs when the 251-byte
> response data are copied from IOMB buffer to response message
> buffer in function mp8001_mpi_set_nvmd_resp. pm8001_mpi_set_nvmd_resp
typo mp8001_mpi_set_nvmd_resp.
> is a function to process the response of command set_nvmd_data_resp.
> After sending the command set_nvmd_data, the caller begins to sleep by
> calling wait_for_complete() and wait for the wake-up from calling
> complete() in pm8001_mpi_set_nvmd_resp. In the current code,
> the memcpy for response message buffer occurs after calling complete().
> So, it is not protected by the use of wait_for_completion() and
> complete().
>
> Due to unexpected events (e.g., interrupt), if response buffer gets
> freed before memcpy, the use-after-free error will occur.
> To fix it, the complete() should be called after memcpy.
>
> Signed-off-by: yuuzheng <yuuzheng@xxxxxxxxxx>
> Signed-off-by: Viswas G <Viswas.G@xxxxxxxxxxxxx>
> Signed-off-by: Ruksar Devadi <Ruksar.devadi@xxxxxxxxxxxxx>
> Signed-off-by: Radha Ramachandran <radha@xxxxxxxxxx>
The code and the commit message doesn't match, the commit message is
talking about pm8001_mpi_set_nvmd_resp,
but the code change is in pm8001_mpi_get_nvmd_resp
The fix itself looks correct, please fix the commit message, with that
you can add my
Acked-by: Jack Wang <jinpu.wang@xxxxxxxxxxxxxxx>
> ---
> drivers/scsi/pm8001/pm8001_hwi.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/pm8001/pm8001_hwi.c b/drivers/scsi/pm8001/pm8001_hwi.c
> index 9e9a546da959..2054c2b03d92 100644
> --- a/drivers/scsi/pm8001/pm8001_hwi.c
> +++ b/drivers/scsi/pm8001/pm8001_hwi.c
> @@ -3279,10 +3279,15 @@ pm8001_mpi_get_nvmd_resp(struct pm8001_hba_info *pm8001_ha, void *piomb)
> pm8001_ha->memoryMap.region[NVMD].virt_ptr,
> fw_control_context->len);
> kfree(ccb->fw_control_context);
> + /* To avoid race condition, complete should be
> + * called after the message is copied to
> + * fw_control_context->usrAddr
> + */
> + complete(pm8001_ha->nvmd_completion);
> + PM8001_MSG_DBG(pm8001_ha, pm8001_printk("Set nvm data complete!\n"));
> ccb->task = NULL;
> ccb->ccb_tag = 0xFFFFFFFF;
> pm8001_tag_free(pm8001_ha, tag);
> - complete(pm8001_ha->nvmd_completion);
> }
>
> int pm8001_mpi_local_phy_ctl(struct pm8001_hba_info *pm8001_ha, void *piomb)
> --
> 2.16.3
>
