Skip to content

Commit c5deb27

Browse files
committed
xen/privcmd: fix error exit of privcmd_ioctl_dm_op()
The error exit of privcmd_ioctl_dm_op() is calling unlock_pages() potentially with pages being NULL, leading to a NULL dereference. Additionally lock_pages() doesn't check for pin_user_pages_fast() having been completely successful, resulting in potentially not locking all pages into memory. This could result in sporadic failures when using the related memory in user mode. Fix all of that by calling unlock_pages() always with the real number of pinned pages, which will be zero in case pages being NULL, and by checking the number of pages pinned by pin_user_pages_fast() matching the expected number of pages. Cc: <stable@vger.kernel.org> Fixes: ab520be ("xen/privcmd: Add IOCTL_PRIVCMD_DM_OP") Reported-by: Rustam Subkhankulov <subkhankulov@ispras.ru> Signed-off-by: Juergen Gross <jgross@suse.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> Link: https://lore.kernel.org/r/20220825141918.3581-1-jgross@suse.com Signed-off-by: Juergen Gross <jgross@suse.com>
1 parent 6bb79f5 commit c5deb27

File tree

1 file changed

+11
-10
lines changed

1 file changed

+11
-10
lines changed

drivers/xen/privcmd.c

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -581,27 +581,30 @@ static int lock_pages(
581581
struct privcmd_dm_op_buf kbufs[], unsigned int num,
582582
struct page *pages[], unsigned int nr_pages, unsigned int *pinned)
583583
{
584-
unsigned int i;
584+
unsigned int i, off = 0;
585585

586-
for (i = 0; i < num; i++) {
586+
for (i = 0; i < num; ) {
587587
unsigned int requested;
588588
int page_count;
589589

590590
requested = DIV_ROUND_UP(
591591
offset_in_page(kbufs[i].uptr) + kbufs[i].size,
592-
PAGE_SIZE);
592+
PAGE_SIZE) - off;
593593
if (requested > nr_pages)
594594
return -ENOSPC;
595595

596596
page_count = pin_user_pages_fast(
597-
(unsigned long) kbufs[i].uptr,
597+
(unsigned long)kbufs[i].uptr + off * PAGE_SIZE,
598598
requested, FOLL_WRITE, pages);
599-
if (page_count < 0)
600-
return page_count;
599+
if (page_count <= 0)
600+
return page_count ? : -EFAULT;
601601

602602
*pinned += page_count;
603603
nr_pages -= page_count;
604604
pages += page_count;
605+
606+
off = (requested == page_count) ? 0 : off + page_count;
607+
i += !off;
605608
}
606609

607610
return 0;
@@ -677,10 +680,8 @@ static long privcmd_ioctl_dm_op(struct file *file, void __user *udata)
677680
}
678681

679682
rc = lock_pages(kbufs, kdata.num, pages, nr_pages, &pinned);
680-
if (rc < 0) {
681-
nr_pages = pinned;
683+
if (rc < 0)
682684
goto out;
683-
}
684685

685686
for (i = 0; i < kdata.num; i++) {
686687
set_xen_guest_handle(xbufs[i].h, kbufs[i].uptr);
@@ -692,7 +693,7 @@ static long privcmd_ioctl_dm_op(struct file *file, void __user *udata)
692693
xen_preemptible_hcall_end();
693694

694695
out:
695-
unlock_pages(pages, nr_pages);
696+
unlock_pages(pages, pinned);
696697
kfree(xbufs);
697698
kfree(pages);
698699
kfree(kbufs);

0 commit comments

Comments
 (0)