[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH RFC] x86: Fix XEN_DOMCTL_gdbsx_guestmemio crash
 
- To: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>
 
- From: Juergen Gross <jgross@xxxxxxxx>
 
- Date: Wed, 20 Apr 2022 18:03:53 +0200
 
- Cc: George Dunlap <George.Dunlap@xxxxxxxxxxxxx>, Jan Beulich <JBeulich@xxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Julien Grall <julien@xxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>, Bertrand Marquis <bertrand.marquis@xxxxxxx>
 
- Delivery-date: Wed, 20 Apr 2022 16:04:22 +0000
 
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
 
 
 
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.cooper3@xxxxxxxxxx>
---
CC: George Dunlap <George.Dunlap@xxxxxxxxxxxxx>
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
CC: Wei Liu <wl@xxxxxxx>
CC: Julien Grall <julien@xxxxxxx>
CC: Juergen Gross <jgross@xxxxxxxx>
CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
CC: Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
CC: Bertrand Marquis <bertrand.marquis@xxxxxxx>
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
 Attachment:
OpenPGP_0xB0DE9DD628BF132F.asc 
Description: OpenPGP public key 
Attachment:
OpenPGP_signature 
Description: OpenPGP digital signature 
 
    
     |