| Message ID | 20220311132206.24515-1-xiaoguang.wang@linux.alibaba.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [1/2] scsi: target: tcmu: Fix possible page UAF | expand |
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; > }
On Fri, 11 Mar 2022 21:22:05 +0800, 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(). > > [...] Applied to 5.18/scsi-fixes, thanks! [1/2] scsi: target: tcmu: Fix possible page UAF https://git.kernel.org/mkp/scsi/c/a6968f7a367f
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; }
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(-)