Re: [PATCH 06/12] pm80xx : Fix dereferencing dangling pointer.

From: Jinpu Wang <jinpu.wang@cloud.ionos.com>
To: Deepak Ukey <deepak.ukey@microchip.com>
Cc: Linux SCSI Mailinglist <linux-scsi@vger.kernel.org>,
	Vasanthalakshmi.Tharmarajan@microchip.com,
	Viswas.G@microchip.com, Jack Wang <jinpu.wang@profitbricks.com>,
	"Martin K. Petersen" <martin.petersen@oracle.com>,
	dpf@google.com, jsperbeck@google.com,
	Vikram Auradkar <auradkar@google.com>,
	ianyar@google.com
Subject: Re: [PATCH 06/12] pm80xx : Fix dereferencing dangling pointer.
Date: Wed, 6 Nov 2019 11:28:15 +0100	[thread overview]
Message-ID: <CAMGffEkXQbGF-Tk4vQWuRpbTG7OcKAqMyHakJVDXBjPT1Z7n5A@mail.gmail.com> (raw)
In-Reply-To: <20191031051241.6762-7-deepak.ukey@microchip.com>

On Thu, Oct 31, 2019 at 6:12 AM Deepak Ukey <deepak.ukey@microchip.com> wrote:
>
> From: Vikram Auradkar <auradkar@google.com>
>
> sas_task structure should not be used after task_done is called.
> If the device is gone or not attached, we call task_done on t and
> continue to use in the sas_task in rest of the function. task_done
> is pointing to sas_ata_task_done, may free the memory associated
> with the task before returning.
>
> Signed-off-by: Vikram Auradkar <auradkar@google.com>
> Signed-off-by: Deepak Ukey <deepak.ukey@microchip.com>
> Signed-off-by: Viswas G <Viswas.G@microchip.com>
Looks fine.
Acked-by: Jack Wang <jinpu.wang@cloud.ionos.com>
> ---
>  drivers/scsi/pm8001/pm8001_sas.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c
> index 447a66d60275..4491de8d40fc 100644
> --- a/drivers/scsi/pm8001/pm8001_sas.c
> +++ b/drivers/scsi/pm8001/pm8001_sas.c
> @@ -388,6 +388,7 @@ static int pm8001_task_exec(struct sas_task *task,
>         struct pm8001_ccb_info *ccb;
>         u32 tag = 0xdeadbeef, rc = 0, n_elem = 0;
>         unsigned long flags = 0;
> +       enum sas_protocol task_proto = t->task_proto;
>
>         if (!dev->port) {
>                 struct task_status_struct *tsm = &t->task_status;
> @@ -412,7 +413,7 @@ static int pm8001_task_exec(struct sas_task *task,
>                 pm8001_dev = dev->lldd_dev;
>                 port = &pm8001_ha->port[sas_find_local_port_id(dev)];
>                 if (DEV_IS_GONE(pm8001_dev) || !port->port_attached) {
> -                       if (sas_protocol_ata(t->task_proto)) {
> +                       if (sas_protocol_ata(task_proto)) {
>                                 struct task_status_struct *ts = &t->task_status;
>                                 ts->resp = SAS_TASK_UNDELIVERED;
>                                 ts->stat = SAS_PHY_DOWN;
> @@ -434,7 +435,7 @@ static int pm8001_task_exec(struct sas_task *task,
>                         goto err_out;
>                 ccb = &pm8001_ha->ccb_info[tag];
>
> -               if (!sas_protocol_ata(t->task_proto)) {
> +               if (!sas_protocol_ata(task_proto)) {
>                         if (t->num_scatter) {
>                                 n_elem = dma_map_sg(pm8001_ha->dev,
>                                         t->scatter,
> @@ -454,7 +455,7 @@ static int pm8001_task_exec(struct sas_task *task,
>                 ccb->ccb_tag = tag;
>                 ccb->task = t;
>                 ccb->device = pm8001_dev;
> -               switch (t->task_proto) {
> +               switch (task_proto) {
>                 case SAS_PROTOCOL_SMP:
>                         rc = pm8001_task_prep_smp(pm8001_ha, ccb);
>                         break;
> @@ -471,8 +472,7 @@ static int pm8001_task_exec(struct sas_task *task,
>                         break;
>                 default:
>                         dev_printk(KERN_ERR, pm8001_ha->dev,
> -                               "unknown sas_task proto: 0x%x\n",
> -                               t->task_proto);
> +                               "unknown sas_task proto: 0x%x\n", task_proto);
>                         rc = -EINVAL;
>                         break;
>                 }
> @@ -495,7 +495,7 @@ static int pm8001_task_exec(struct sas_task *task,
>         pm8001_tag_free(pm8001_ha, tag);
>  err_out:
>         dev_printk(KERN_ERR, pm8001_ha->dev, "pm8001 exec failed[%d]!\n", rc);
> -       if (!sas_protocol_ata(t->task_proto))
> +       if (!sas_protocol_ata(task_proto))
>                 if (n_elem)
>                         dma_unmap_sg(pm8001_ha->dev, t->scatter, t->num_scatter,
>                                 t->data_dir);
> --
> 2.16.3
>