Re: [PATCH 3/3] s390/pci: Add function handle to RPCIT insn trace

[Gerd Bayer <gbayer@linux.ibm.com>]

On Wed, 2026-10-07 at 11:17 +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] s390/pci: info leak of uninitialized struct padding in zpci_err_insn_rpcit
> --
> 
> commit 9deafec98847a2a1de20826fbaa6431e1e3c973f
> Author: Gerd Bayer <gbayer@linux.ibm.com>
> 
> s390/pci: Add function handle to RPCIT insn trace
> 
> In certain debug situations it would be helpful, if the insn trace for a
> failing RPCIT would reveal the PCI function on which this was attempted.
> 
> Introduce a variant of zpci_err_insn_req() called zpci_err_insn_rpcit()
> that accepts an IO virtual address and a range parameter together with
> the req conveying the function handle. Switch zpci_refresh_trans() to
> use that new error trace.
> 
> > diff --git a/arch/s390/pci/pci_insn.c b/arch/s390/pci/pci_insn.c
> > index 053afb3e7a990..ae9ed4aa2ec9c 100644
> > --- a/arch/s390/pci/pci_insn.c
> > +++ b/arch/s390/pci/pci_insn.c
> 
> [ ... ]
> 
> > @@ -29,18 +29,40 @@ struct zpci_err_insn_data {
> >  	union {
> >  		struct {
> >  			u64 req;
> > -			u64 offset;
> > +			union {
> > +				/* PCILG/PCISTG/MPCIFC */
> > +				struct {
> > +					u64 offset;
> > +					u64: 64;
> > +				};
> > +				/* RPCIT */
> > +				struct {
> > +					u64 iova;
> > +					u64 range;
> > +				};
> > +			};
> >  		};
> > +		/* PCILGI/PCISTGI */
> >  		struct {
> >  			u64 addr;
> >  			u64 len;
> > +			u64: 64;
> >  		};
> >  	};
> > -	u64: 64;
> >  };
> >  
> >  static_assert(sizeof(struct zpci_err_insn_data) <= ZPCI_ERR_INSN_DATA_SIZE);
> >  
> > +static inline void zpci_err_insn_rpcit(int lvl, u8 insn, u8 cc, u8 status,
> > +				       u64 req, u64 iova, u64 range)
> > +{
> > +	struct zpci_err_insn_data data = {
> > +		.insn = insn, .cc = cc, .status = status,
> > +		.req = req, .iova = iova, .range = range};
> > +
> > +	zpci_err_hex_level(lvl, &data, sizeof(data));
> > +}
> 
> [Severity: Medium]
> Does this code leak uninitialized stack memory to the s390dbf debug trace
> buffer?
> 
> According to the C standard, unnamed bit-fields do not participate in
> designated initialization. The struct zpci_err_insn_data starts with:
> 
> struct zpci_err_insn_data {
> 	u8 insn;
> 	u8 cc;
> 	u8 status;
> 	u8: 8;
> 	u32: 32;
>     ...
> 
> Because the unnamed bit-fields are skipped by the designated initializer here,
> up to 5 bytes of uninitialized kernel stack memory are left in the padding
> of "data".
> 
> Since sizeof(data) (which is 32 bytes) is then passed to zpci_err_hex_level(),
> could these indeterminate stack bytes be copied into the trace buffer on every
> invocation of zpci_err_insn_rpcit()?

This is basically the same concern as with patch 1/3 - I'll replace the
unnamed bit-fields with dummy members of appropriate size in a v2.