Re: [PATCH RFC] x86: Fix XEN_DOMCTL_gdbsx_guestmemio crash

[Juergen Gross]

On 20.04.22 17:56, Andrew Cooper wrote:

When CONFIG_GDBSX is compiled out, iommu_do_domctl() falls over a NULL
pointer.  It isn't really correct for processing of XEN_DOMCTL_gdbsx_* to fall
into the default case when compiled out.

Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com>
---
CC: George Dunlap <george.dun...@eu.citrix.com>
CC: Jan Beulich <jbeul...@suse.com>
CC: Stefano Stabellini <sstabell...@kernel.org>
CC: Wei Liu <w...@xen.org>
CC: Julien Grall <jul...@xen.org>
CC: Juergen Gross <jgr...@suse.com>
CC: Roger Pau Monné <roger....@citrix.com>
CC: Volodymyr Babchuk <volodymyr_babc...@epam.com>
CC: Bertrand Marquis <bertrand.marq...@arm.com>

RFC, because this has implications across the codebase.  The tl;dr is that
case FOO:'s shouldn't be compiled out; we still know what the subops are, even
when the functionality is compiled out.

There are several ways to express this.  Alternatives would be:

     case XEN_DOMCTL_gdbsx_guestmemio:
         if ( !IS_ENABLED(CONFIG_GDBSX) )
         {
             rc = -EOPNOTSUPP;
             break;
         }
         ...;

but given my debugger series creating gdbsx.c, I was also considering:

     case XEN_DOMCTL_gdbsx_guestmemio:
     case XEN_DOMCTL_gdbsx_pausevcpu:
     case XEN_DOMCTL_gdbsx_unpausevcpu:
     case XEN_DOMCTL_gdbsx_domstatus:
         rc = gdbsx_do_domctl(d, iop);
         break;

I'd go this route.


Juergen

OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

OpenPGP_signature
Description: OpenPGP digital signature