The error exit of privcmd_ioctl_dm_op() is calling unlock_pages() potentially with pages being NULL, leading to a NULL dereference.
Fix that by calling unlock_pages only if lock_pages() was at least partially successful.
Cc: stable@vger.kernel.org Fixes: ab520be8cd5d ("xen/privcmd: Add IOCTL_PRIVCMD_DM_OP") Reported-by: Rustam Subkhankulov subkhankulov@ispras.ru Signed-off-by: Juergen Gross jgross@suse.com --- drivers/xen/privcmd.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c index 3369734108af..ec87968b4459 100644 --- a/drivers/xen/privcmd.c +++ b/drivers/xen/privcmd.c @@ -679,7 +679,7 @@ static long privcmd_ioctl_dm_op(struct file *file, void __user *udata) rc = lock_pages(kbufs, kdata.num, pages, nr_pages, &pinned); if (rc < 0) { nr_pages = pinned; - goto out; + goto unlock; }
for (i = 0; i < kdata.num; i++) { @@ -691,8 +691,9 @@ static long privcmd_ioctl_dm_op(struct file *file, void __user *udata) rc = HYPERVISOR_dm_op(kdata.dom, kdata.num, xbufs); xen_preemptible_hcall_end();
-out: + unlock: unlock_pages(pages, nr_pages); + out: kfree(xbufs); kfree(pages); kfree(kbufs);
On 24.08.22 17:26, Juergen Gross wrote:
Hello Juergen
Reviewed-by: Oleksandr Tyshchenko oleksandr_tyshchenko@epam.com
On 24.08.2022 16:26, Juergen Gross wrote:
Reviewed-by: Jan Beulich jbeulich@suse.com albeit I wonder whether you did consider the variant actually reducing code size (and avoiding the need for yet another label), ...
... dropping this line and ...
... passing "pinned" here.
Jan
On 25.08.22 09:38, Jan Beulich wrote:
Looking into this I found another problem: NOT using pinned is wrong, as lock_pages() doesn't guarantee that all pages were really locked. I think lock_pages() should return an error, in case pin_user_pages_fast() didn't lock as many pages es expected.
Juergen
linux-stable-mirror@lists.linaro.org
-
Jan Beulich -
Juergen Gross -
Oleksandr Tyshchenko