[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] xen/sysctl: fix XEN_SYSCTL_getdomaininfolist handling with XSM


  • To: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • From: "Daniel P. Smith" <dpsmith@xxxxxxxxxxxxxxxxxxxx>
  • Date: Tue, 2 May 2023 09:17:33 -0400
  • Arc-authentication-results: i=1; mx.zohomail.com; dkim=pass header.i=apertussolutions.com; spf=pass smtp.mailfrom=dpsmith@xxxxxxxxxxxxxxxxxxxx; dmarc=pass header.from=<dpsmith@xxxxxxxxxxxxxxxxxxxx>
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1683033455; h=Content-Type:Content-Transfer-Encoding:Cc:Date:From:In-Reply-To:MIME-Version:Message-ID:References:Subject:To; bh=Bj4+tCe/Djy4x77OBozlm/+/KUc/w8n08KGpXcqAFa0=; b=PfzrMRF/rh23CiCZobBufEefvheYPU8Fgodc/pFxKVuVUhu406UagzdU9voi/3QdAp0WFl2VVNZ85deMgIjMllE3ilpnJ5aVXrAHgWrA/VRea7cQh0cpmFbxD9LSE3g0PHNQUtnIWio1BOyO7zr7GYMADceR0xa65Z6ppMFBR0I=
  • Arc-seal: i=1; a=rsa-sha256; t=1683033455; cv=none; d=zohomail.com; s=zohoarc; b=bKgVqM1cUjQg1RE4Z6Kx69DSsRAw4rMDqvv6/Sy2O23QeM5wbXrA0LI7luRdXAA/4dQw/9Ws5WwK9qMhCm1a8suuRhTktwnWhJwHo9YS5fF+o8M7ojwnViVEQTy17sXfZw7idmwx7uuC6KslLYd3sXcfZ7W/IfaUsqTeBLh39jI=
  • Cc: Juergen Gross <jgross@xxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>, Julien Grall <julien@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>
  • Delivery-date: Tue, 02 May 2023 13:17:42 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 5/2/23 09:10, Roger Pau Monné wrote:
On Tue, May 02, 2023 at 09:03:00AM -0400, Daniel P. Smith wrote:
On 4/30/23 10:46, Juergen Gross wrote:
In case XSM is active, the handling of XEN_SYSCTL_getdomaininfolist
can fail if the last domain scanned isn't allowed to be accessed by
the calling domain (i.e. xsm_getdomaininfo(XSM_HOOK, d) is failing).

Fix that by just ignoring scanned domains where xsm_getdomaininfo()
is returning an error, like it is effectively done when such a
situation occurs for a domain not being the last one scanned.

Fixes: d046f361dc93 ("Xen Security Modules: XSM")
Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
---
   xen/common/sysctl.c | 3 +--
   1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
index 02505ab044..0cbfe8bd44 100644
--- a/xen/common/sysctl.c
+++ b/xen/common/sysctl.c
@@ -89,8 +89,7 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
               if ( num_domains == op->u.getdomaininfolist.max_domains )
                   break;
-            ret = xsm_getdomaininfo(XSM_HOOK, d);
-            if ( ret )
+            if ( xsm_getdomaininfo(XSM_HOOK, d) )
                   continue;
               getdomaininfo(d, &info);


This change does not match the commit message. This says it fixes an issue,
but unless I am totally missing something, this change is nothing more than
formatting that drops the use of an intermediate variable. Please feel free
to correct me if I am wrong here, otherwise I believe the commit message
should be changed to reflect the code change.

By dropping that intermediate variable it prevents returning an error
as the result of the hypercall if xsm_getdomaininfo() for the last
domain fails.

Ah, understood. I missed ret is state tracking.

Note that xsm_getdomaininfo() failing for other domains not the last
one don't cause the return value of the hypercall to be an error
code, because the variable containing the error gets overwritten by
further loops.

In the end, this is just addressing an issue that has not been seen by anyone and happened upon while debugging another issue.

V/r,
DPS



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.