Re: [PATCH net-next v5 2/2] net: dsa: mxl862xx: implement bridge offloading
From: Daniel Golle
Date: Thu Mar 19 2026 - 08:02:57 EST
On Thu, Mar 19, 2026 at 09:51:40AM +0100, Paolo Abeni wrote:
> On 3/15/26 5:20 PM, Daniel Golle wrote:
> > +static int mxl862xx_set_ageing_time(struct dsa_switch *ds, unsigned int msecs)
> > +{
> > + struct mxl862xx_cfg param;
> > + int ret;
> > +
> > + ret = MXL862XX_API_READ(ds->priv, MXL862XX_COMMON_CFGGET, param);
>
> AI review says:
>
> Is the param struct initialized here? Looking at MXL862XX_API_READ(), it
> expands to mxl862xx_api_wrap() which writes the contents of the data buffer
> to firmware MDIO registers before sending the GET command. This would send
> uninitialized kernel stack data to the firmware hardware.
>
> Every other MXL862XX_API_READ() call site in the driver uses a
> zero-initialized struct. For example, in mxl862xx_setup():
>
> struct mxl862xx_cfg cfg = {};
> ...
> ret = MXL862XX_API_READ(priv, MXL862XX_COMMON_CFGGET, cfg);
>
> Should param be initialized with = {} here?
>
While I agree that just sending over uninitialized memory is kinda ugly,
for this specific firmware API call it is not a problem because it
completely discards what ever is sent, and fully overwrites all
struct members before sending it back to the host.
Anyway zero-initializing it with {} is probably still good practise
and I'll do that, so AI or static analysis tools don't complain about
that.