Re: [PATCH 1/2] scsi: target: tcmu: Fix possible page UAF

[Bodo Stroesser <bostroesser@gmail.com>]

This one looks good. Thank you.

Reviewed-by: Bodo Stroesser <bostroesser@gmail.com>


On 11.03.22 14:22, Xiaoguang Wang wrote:
> tcmu_try_get_data_page() looks up pages under cmdr_lock, but it don't
> take refcount properly and just return page pointer.
> 
> When tcmu_try_get_data_page() returns, the returned page may have been
> freed by tcmu_blocks_release(), need to get_page() under cmdr_lock to
> avoid concurrent tcmu_blocks_release().
> 
> Signed-off-by: Xiaoguang Wang <xiaoguang.wang@linux.alibaba.com>
> ---
>   drivers/target/target_core_user.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/target/target_core_user.c b/drivers/target/target_core_user.c
> index 7b2a89a67cdb..06a5c4086551 100644
> --- a/drivers/target/target_core_user.c
> +++ b/drivers/target/target_core_user.c
> @@ -1820,6 +1820,7 @@ static struct page *tcmu_try_get_data_page(struct tcmu_dev *udev, uint32_t dpi)
>   	mutex_lock(&udev->cmdr_lock);
>   	page = xa_load(&udev->data_pages, dpi);
>   	if (likely(page)) {
> +		get_page(page);
>   		mutex_unlock(&udev->cmdr_lock);
>   		return page;
>   	}
> @@ -1876,6 +1877,7 @@ static vm_fault_t tcmu_vma_fault(struct vm_fault *vmf)
>   		/* For the vmalloc()ed cmd area pages */
>   		addr = (void *)(unsigned long)info->mem[mi].addr + offset;
>   		page = vmalloc_to_page(addr);
> +		get_page(page);
>   	} else {
>   		uint32_t dpi;
>   
> @@ -1886,7 +1888,6 @@ static vm_fault_t tcmu_vma_fault(struct vm_fault *vmf)
>   			return VM_FAULT_SIGBUS;
>   	}
>   
> -	get_page(page);
>   	vmf->page = page;
>   	return 0;
>   }