Re: [PATCH RFC] x86: Fix XEN_DOMCTL_gdbsx_guestmemio crash

[Jan Beulich <jbeulich@xxxxxxxx>]

On 20.04.2022 18:03, Juergen Gross wrote:
> 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.

+1 if we already start enumerating sub-system domctl-s (as proposed
for the IOMMU ones as well).

Jan