[SRU][Xenial][CVE-2018-20836][PATCH] scsi: libsas: fix a race condition when smp task timeout

[Kleber Souza]

On 5/31/19 12:31 AM, Connor Kuehl wrote:
> From: Jason Yan <yanaijie at huawei.com>
> 
> CVE-2018-20836
> 
> BugLink: https://bugs.launchpad.net/bugs/1808912

Hi Connor,

You have added the above BugLink as well for this CVE fix but this bug
report has been marked as invalid for Xenial.

Generally we don't need to add a BugLink for a CVE fix, the CVE ID
is enough. Is this BugLink really needed?


Thanks,
Kleber

> 
> When the lldd is processing the complete sas task in interrupt and set the
> task stat as SAS_TASK_STATE_DONE, the smp timeout timer is able to be
> triggered at the same time. And smp_task_timedout() will complete the task
> wheter the SAS_TASK_STATE_DONE is set or not. Then the sas task may freed
> before lldd end the interrupt process. Thus a use-after-free will happen.
> 
> Fix this by calling the complete() only when SAS_TASK_STATE_DONE is not
> set. And remove the check of the return value of the del_timer(). Once the
> LLDD sets DONE, it must call task->done(), which will call
> smp_task_done()->complete() and the task will be completed and freed
> correctly.
> 
> Reported-by: chenxiang <chenxiang66 at hisilicon.com>
> Signed-off-by: Jason Yan <yanaijie at huawei.com>
> CC: John Garry <john.garry at huawei.com>
> CC: Johannes Thumshirn <jthumshirn at suse.de>
> CC: Ewan Milne <emilne at redhat.com>
> CC: Christoph Hellwig <hch at lst.de>
> CC: Tomas Henzl <thenzl at redhat.com>
> CC: Dan Williams <dan.j.williams at intel.com>
> CC: Hannes Reinecke <hare at suse.com>
> Reviewed-by: Hannes Reinecke <hare at suse.com>
> Reviewed-by: John Garry <john.garry at huawei.com>
> Reviewed-by: Johannes Thumshirn <jthumshirn at suse.de>
> Signed-off-by: Martin K. Petersen <martin.petersen at oracle.com>
> (cherry picked from commit b90cd6f2b905905fb42671009dc0e27c310a16ae)
> Signed-off-by: Connor Kuehl <connor.kuehl at canonical.com>
> ---
>  drivers/scsi/libsas/sas_expander.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
> index 7be581f7c35d..1a6f65db615e 100644
> --- a/drivers/scsi/libsas/sas_expander.c
> +++ b/drivers/scsi/libsas/sas_expander.c
> @@ -47,17 +47,16 @@ static void smp_task_timedout(unsigned long _task)
>  	unsigned long flags;
>  
>  	spin_lock_irqsave(&task->task_state_lock, flags);
> -	if (!(task->task_state_flags & SAS_TASK_STATE_DONE))
> +	if (!(task->task_state_flags & SAS_TASK_STATE_DONE)) {
>  		task->task_state_flags |= SAS_TASK_STATE_ABORTED;
> +		complete(&task->slow_task->completion);
> +	}
>  	spin_unlock_irqrestore(&task->task_state_lock, flags);
> -
> -	complete(&task->slow_task->completion);
>  }
>  
>  static void smp_task_done(struct sas_task *task)
>  {
> -	if (!del_timer(&task->slow_task->timer))
> -		return;
> +	del_timer(&task->slow_task->timer);
>  	complete(&task->slow_task->completion);
>  }
>  
>