From nobody Tue May 21 17:07:01 2024 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 44ADFC28D13 for ; Thu, 25 Aug 2022 11:40:14 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236932AbiHYLkM (ORCPT ); Thu, 25 Aug 2022 07:40:12 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33684 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236917AbiHYLkI (ORCPT ); Thu, 25 Aug 2022 07:40:08 -0400 Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A322A25EB3; Thu, 25 Aug 2022 04:40:07 -0700 (PDT) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id 2240D33FAB; Thu, 25 Aug 2022 11:40:06 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1661427606; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding; bh=dwYP6pNMJ0bm8xlxtVpMlGoTLHlcq559EPLF7XX0wWg=; b=C+MPh8mLFnN9CLM7FjKQOWdgVZYRDFXFB5CoDv9yeClqcY0jYdkWN9NppQSYqOGtsFRPo9 llIVVfYKHqCMkvMFoXXzJLHOUxQsCIuE1TSrgfoXYc80a6jPEMgghctgM2MdCM7R3fZLqu fsBv5JQ9zYRfGwnkpB3TE73sX55oOoM= Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id D754213517; Thu, 25 Aug 2022 11:40:05 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id wBBAM5VfB2OhfwAAMHmgww (envelope-from ); Thu, 25 Aug 2022 11:40:05 +0000 From: Juergen Gross To: xen-devel@lists.xenproject.org, linux-kernel@vger.kernel.org Cc: Juergen Gross , Stefano Stabellini , Oleksandr Tyshchenko , stable@vger.kernel.org, Rustam Subkhankulov Subject: [PATCH v3] xen/privcmd: fix error exit of privcmd_ioctl_dm_op() Date: Thu, 25 Aug 2022 13:40:04 +0200 Message-Id: <20220825114004.24843-1-jgross@suse.com> X-Mailer: git-send-email 2.35.3 MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Type: text/plain; charset="utf-8" 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: Fixes: ab520be8cd5d ("xen/privcmd: Add IOCTL_PRIVCMD_DM_OP") Reported-by: Rustam Subkhankulov Signed-off-by: Juergen Gross --- V2: - use "pinned" as parameter for unlock_pages() (Jan Beulich) - drop label "unlock" again (Jan Beulich) - add check for complete success of pin_user_pages_fast() V3: - continue after partial success of pin_user_pages_fast() (Jan Beulich) --- drivers/xen/privcmd.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c index 3369734108af..1ca7e3ea6fd4 100644 --- a/drivers/xen/privcmd.c +++ b/drivers/xen/privcmd.c @@ -581,7 +581,7 @@ static int lock_pages( struct privcmd_dm_op_buf kbufs[], unsigned int num, struct page *pages[], unsigned int nr_pages, unsigned int *pinned) { - unsigned int i; + unsigned int i, off =3D 0; =20 for (i =3D 0; i < num; i++) { unsigned int requested; @@ -589,19 +589,23 @@ static int lock_pages( =20 requested =3D DIV_ROUND_UP( offset_in_page(kbufs[i].uptr) + kbufs[i].size, - PAGE_SIZE); + PAGE_SIZE) - off; if (requested > nr_pages) return -ENOSPC; =20 page_count =3D pin_user_pages_fast( - (unsigned long) kbufs[i].uptr, + (unsigned long)kbufs[i].uptr + off * PAGE_SIZE, requested, FOLL_WRITE, pages); - if (page_count < 0) - return page_count; + if (page_count <=3D 0) + return page_count ? : -EFAULT; =20 *pinned +=3D page_count; nr_pages -=3D page_count; pages +=3D page_count; + + off =3D requested - page_count; + if (off) + i--; } =20 return 0; @@ -677,10 +681,8 @@ static long privcmd_ioctl_dm_op(struct file *file, voi= d __user *udata) } =20 rc =3D lock_pages(kbufs, kdata.num, pages, nr_pages, &pinned); - if (rc < 0) { - nr_pages =3D pinned; + if (rc < 0) goto out; - } =20 for (i =3D 0; i < kdata.num; i++) { set_xen_guest_handle(xbufs[i].h, kbufs[i].uptr); @@ -692,7 +694,7 @@ static long privcmd_ioctl_dm_op(struct file *file, void= __user *udata) xen_preemptible_hcall_end(); =20 out: - unlock_pages(pages, nr_pages); + unlock_pages(pages, pinned); kfree(xbufs); kfree(pages); kfree(kbufs); --=20 2.35.3