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

[sashiko-bot@kernel.org]

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()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261007-rpcit_trcfh_upstream-v1-0-8a2718cfb90b@linux.ibm.com?part=3