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

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


  • To: "Daniel P. Smith" <dpsmith@xxxxxxxxxxxxxxxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Tue, 2 May 2023 15:10:07 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=YnyaevPHt9hCzAcJesQfdl7YN/uqEVelGTTDFPY2XNk=; b=lGuhUQK+M6uSqWMO/N0NnDf5XPXAlt1y4eLybdpUTxRa596ZWpFFtjDnkDCyJMo5v8pxPgnjzY7Uke2VCyHDAsNRlDU3rfiSorBo7Y5iV+ibA4SBJB8OwS08Ttt8npfNbfjuu+ofmwNEaUbYG4MXhcJTgwjn8mATAUS8I57VZazBkrJa2T3iYRHWR03jH4P21GaHWZSkUliJ7bs3bTfUZi4H/vbfdyhp6MRDJBaVCQSWYxTKSQ+M0AIZyiUl8sE8UFba8bDUbSyphzr183xxqV+7FajZBDnqdqcWu58/ulQTac/4okl/X/I2P/p6JnlOg8M9EEY0xs6odCbNabD0tA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=lFyDtxFMI5BcUyRjwWhOm6ZQUx9JZ+CZdN8MukvFFQSjUdtqgCVWlJVlAcDjqNs8/TRjXVzPadrcATJnAd/g21GqAWu5EmKmaKU7VAC/n8D5lnohjg6qm51aYVfWqJPyPmVpCo3ggRyecGpoPa3+igJWiX9/AZR7mMU3r9+hbVDpNTC6yMKjGbjoYaATrc5bDqRMad0I0UswgXA2QIRTZU4xQViMv/yGn1Ib7b/ORydt5iOWkGti8X7gEWqgH9HaQNVtA32VD4HfnGE1jOC1VSSWth1xWOFlbUgnpOYTLpI45NjLZml0KGaEkd77Dq57SSepxf2V0DCMGWcoKeg0sQ==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
  • 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:10:34 +0000
  • Ironport-data: A9a23:7oYHTqACK8wkChVW/9biw5YqxClBgxIJ4kV8jS/XYbTApDon0DdTn GIbWjrQa/mDNDOmed1zb4TnoRgD6pPWyYBqQQY4rX1jcSlH+JHPbTi7wuUcHAvJd5GeExg3h yk6QoOdRCzhZiaE/n9BCpC48T8nk/nOHuGmYAL9EngZbRd+Tys8gg5Ulec8g4p56fC0GArIs t7pyyHlEAbNNwVcbyRFuspvlDs15K6p4G5A5ARkDRx2lAS2e0c9Xcp3yZ6ZdxMUcqEMdsamS uDKyq2O/2+x13/B3fv8z94X2mVTKlLjFVDmZkh+AsBOsTAbzsAG6Y4pNeJ0VKtio27hc+ada jl6ncfYpQ8BZsUgkQmGOvVSO3kW0aZuoNcrLZUj2CA6IoKvn3bEmp1T4E8K0YIw/aFrP15Bx /gkMW4xbE7fhaWI2rS+Rbw57igjBJGD0II3nFhFlGicIdN4BJfJTuPN+MNS2yo2ioZWB/HCa sEFaD1pKhPdfxlIPVRRA5U79AuqriCnL3sE9xTI+uxuvDe7IA9ZidABNPLPfdOHX4NNl1uwr WPa5WXpRBodMbRzzBLcqiz33beewHKTtIQ6L6Hk3ORQhEyowHEuBxEcd0DgreGzhRvrMz5YA wlOksY0loA+7FeuT8X9dxSgrWSYowUHXN5ND+w97hrLwa3Riy6bCXIDVSVpc8E9uYk9QjlC/ k+EmZblCCJitJWRSGmB7fGEoDWqIy8XIGQeIygeQmMt4dPuvYUyhRLnVct4Hei+ididMTPtx zGHqgAuirNVitQEv42g5kzOiT+oopnPTyY26x/RU2bj6Rl2DKa6Y6S45F6d6uxPRLt1VXGEt XkA3sSbsuYHCMjUkDTXGbpSWra0+/yCLTvQx0Z1GIUs/Cis/Hjlep1M5DZ5JwFiNcNslSLVX XI/cDh5vPd7VEZGp4cuC25tI6zGFZTdKOk=
  • Ironport-hdrordr: A9a23:H+bZBqmszeS+IVs//9cWkMZOspXpDfIN3DAbv31ZSRFFG/Fw9v re5cjzsCWVtN9/YgBCpTntAtjjfZqkz+8S3WBzB8bGYOCZghrTEGgK1+KLqAEIfReOjtK1vp 0QFJSWZueAdmSTiq7BjjVRRL4brOVu4siT5Ns33B9WIj2De8lbhTuQEG6gf3Ge7TM2YaYEKA ==
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

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.

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.

Regards, Roger.



 


Rackspace

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