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

Re: [PATCH] x86/cet: Support cet=<bool> on the command line


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Fri, 29 Apr 2022 10: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=VfXZbNMjIvgW4jqo0FcHbguhQKNiLhEPP2OQCR7DD7I=; b=k/KVpRB6elItLBTE7QrQ75nHAmhneQxR5C2f0OYMknNhg067KXwvi52DlYWa4JzHIVbUAImiXp8pgXbij4hyTbxoFUu6GiQu6JTamABIvE0MO19A1SRbBi4GsI6ZxrRxihRWH9mG43mVXm/RnIENIIO2eg+xhtzfHK9w4I9batHgKNkkwOsbsoc2HE8DvMXb06xH/jEYMRztXner16gjlLyYj11jv3LqzyKOxC6DRCOdhq3wSd4oFNbDGEKV7ob7GcM02EnNCPtZEp07+W4CqOEI9HMQwQxwqCT0aL+Jrny06ErPuz1Knjk/ThkTWg61k9S8UVqUMecM3+KwN7ExFw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=irGbea6lB8ijBfIcamUvlgeBRnSR0pSSL/qDdP98Fi2RYVyUI7Rh8vU3guFcGCt37CtS67zrNi9JQpyf9jd4CJf6HIjM/VAdAlYHPLZzZ68r/Y+giRg/1bocK7TkwCXMH3i/mvDPEM/OUgG5Bpv0WuBvVM/RjKg88EpkTqgFLCDT9FANQvMpsFq56IvJQE1xSRuznYFPoAfBYuss3eiQrWOps/KYUunEWmDV9qfnKiOdVwpYSSgK+LELow8ioSZUsZNT1Lc62KGqQMDPr4CFvPxNkl76CsoEsYo48A7CcVXRlqxpY48YxKUuw+DSCrYwsdmPtRyvsjJkoZbsEyH1jg==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
  • Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Fri, 29 Apr 2022 08:10:27 +0000
  • Ironport-data: A9a23:U7g7+avBo7k2hr9GG2x6VRpaAOfnVCJfMUV32f8akzHdYApBsoF/q tZmKT+OM/aDZGf3fd13aN+2pE8B7ZTSy4QyTApuqXwwRHgQ+JbJXdiXEBz9bniYRiHhoOOLz Cm8hv3odp1coqr0/0/1WlTZhSAgk/nOHNIQMcacUsxLbVYMpBwJ1FQywobVvqYy2YLjW1/X6 IuryyHiEATNNwBcYzp8B52r8HuDjNyq0N/PlgVjDRzjlAa2e0g9VPrzF4noR5fLatA88tqBb /TC1NmEElbxpH/BPD8HfoHTKSXmSpaKVeSHZ+E/t6KK2nCurQRquko32WZ1he66RFxlkvgoo Oihu6BcRi9yOpPFqbsxbiNKTRNVBIoX95rpInSg5Jn7I03uKxMAwt1IJWRvZcg037gyBmtDs /sFNDoKcxaPwfqsx662QfVtgcJlK9T3OIQYuTdryjSx4fQOGMifBfmVo4IImm5o2aiiHt6HD yYdQSBoYxnaJQVGJ38cCY4knffujX76G9FdgAzE+/JnvjaNpOB3+KHUHsTLZc2HfNRcumfIn zqd7UO+Ow5PYbRzzhLAqBpAnNTnnyn2RYYTH72Q7eNxjRuYwWl7IAISfUu2p7++kEHWc8JSL QkY9zQjqYA29Ve3VZ/tUhugunmGsxUAHd1KHIUHBBqlz6PV50OVAzYCRzsYMNg+7pZuGnoty 0ODmM7vCXp3qrqJRHmB97CS6zSvJSwSKmxEbigBJecY3+TeTEgIpkqnZr5e/GSd17UZxRmYL +i2kRUD
  • Ironport-hdrordr: A9a23:vFSlYqBBxXzwwRjlHeg/sceALOsnbusQ8zAXPh9KJCC9I/bzqy nxpp8mPH/P5wr5lktQ/OxoHJPwOU80lKQFmLX5WI3PYOCIgguVxe1ZnOjfKnjbalbDH41mpN tdmspFebrN5DFB5K6VgTVQUexQpuVvmJrY+Ns2pE0dKT2CBZsQjTuQXW2gYzdLrUR9dNMEPa vZwvACiyureHwRYMj+Ln4ZX9Lbr9mOsJ79exYJCzMu9QHL1FqTmfXHOind+i1bfyJEwL8k/2 SAuwvl5p+7u/X+7hPHzWfc47lfhdOk4NpeA86njNQTN1zX+06VTbUkf4fHkCE+oemp5lpvuN 7Qoy04N8A20H/VdnHdm2qY5+FNuAxem0PK+Bu9uz/OsMb5TDU1B45qnoRCaCbU7EImoZVVzL 9L93jxjesZMTrw2ADGo/TYXRBjkUS55VA4l/QIsnBZWYwCLJdMsI0k+l9PGptoJlO31GkeKp guMCjg3ocXTbvDBEqp/VWHgebcE0jbJy32DHTr4aeuonprdHMQ9Tps+CVQpAZEyHsHceg72w 31CNUWqFhwdL5mUUtcPpZ3fSLlMB26ffrzWFjiUmjPJeUgB0/njaLRzfEc2NyKEaZ4vqfa3q 6xGm9liQ==
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Thu, Apr 28, 2022 at 12:13:31PM +0200, Jan Beulich wrote:
> On 28.04.2022 10:52, Andrew Cooper wrote:
> > @@ -283,6 +283,8 @@ CET is incompatible with 32bit PV guests.  If any CET 
> > sub-options are active,
> >  they will override the `pv=32` boolean to `false`.  Backwards compatibility
> >  can be maintained with the pv-shim mechanism.
> >  
> > +*   An unqualified boolean is shorthand for setting all suboptions at once.
> 
> You're the native speaker, but I wonder whether there an "a" missing
> before "shorthand".
> 
> > --- a/xen/arch/x86/setup.c
> > +++ b/xen/arch/x86/setup.c
> > @@ -117,7 +117,20 @@ static int __init cf_check parse_cet(const char *s)
> >          if ( !ss )
> >              ss = strchr(s, '\0');
> >  
> > -        if ( (val = parse_boolean("shstk", s, ss)) >= 0 )
> > +        if ( (val = parse_bool(s, ss)) >= 0 )
> > +        {
> > +#ifdef CONFIG_XEN_SHSTK
> > +            opt_xen_shstk = val;
> > +#else
> > +            no_config_param("XEN_SHSTK", "cet", s, ss);
> > +#endif
> > +#ifdef CONFIG_XEN_IBT
> > +            opt_xen_ibt = val;
> > +#else
> > +            no_config_param("XEN_IBT", "cet", s, ss);
> > +#endif
> 
> There shouldn't be two invocations of no_config_param() here; imo if
> either CONFIG_* is defined, use of the option shouldn't produce any
> warning at all.

Hm, I think we would want to warn if someone sets cet=1 but some of
the options have not been built in?  Or else a not very conscious
administrator might believe that all CET options are enabled when some
might not be present in the build.  This would also assume that all
options are positive.

IMO the current approach doesn't seem bad to me, I think it's always
better to error on the side of printing too verbose information rather
than omitting it, specially when it's related to user input on the
command line and security sensitive options.

Thanks, Roger.



 


Rackspace

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