Re: [PATCH 09/28] mfd: cgbc: Zero-initialise version buffer in cgbc_get_version()

[Thomas Richard (congatec GmbH)]

> If the Board Controller returns a short response to the firmware
> revision query in cgbc_get_version(), cgbc_command() can succeed
> without populating all bytes of the stack-allocated data buffer.
> Those uninitialised stack bytes are then copied into cgbc->version
> and exposed to userspace via the cgbc_version sysfs attribute.
> 
> Zero-initialise the data buffer on the stack to prevent leaking
> uninitialised kernel stack memory.
> 
> Fixes: 6f1067cfbee7 ("mfd: Add Congatec Board Controller driver")
> Closes: https://sashiko.dev/#/bug/linux-8c473713-527e-4619-8404-689f28c7654d
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Lee Jones <lee@xxxxxxxxxx>
>
> diff --git a/drivers/mfd/cgbc-core.c b/drivers/mfd/cgbc-core.c
> index edfac2b60718..7513ba42eb96 100644
> --- a/drivers/mfd/cgbc-core.c
> +++ b/drivers/mfd/cgbc-core.c
> @@ -461,7 +461,7 @@ ATTRIBUTE_GROUPS(cgbc);
>  static int cgbc_get_version(struct cgbc_device_data *cgbc)
>  {
>  	u8 cmd = CGBC_CMD_GET_FW_REV;
> -	u8 data[4];
> +	u8 data[4] = {};
>  	int ret;
>  
>  	ret = cgbc_command(cgbc, &cmd, 1, &data, sizeof(data), NULL);

Yes Sashiko complained multiple times even if I already explained it cannot
happen if cgbc_command succeeds. Anyway I agree that things can be improved
but I would prefer in a generic way:
- add macros for cmd resp size (eg CGBC_CMD_GET_FW_REV_RESP_SIZE) in all cgbc
  drivers
- cgbc_command() should return an error if data buffer size doesn't match
  the number of bytes returned by the hardware, it is not the case today [1].

[1] https://elixir.bootlin.com/linux/v7.3/source/drivers/mfd/cgbc-core.c#L194-L195

I'll prepare a series if you are okay.

Best Regards,
Thomas

-- 
Thomas Richard (congatec GmbH) <thomas.richard@xxxxxxxxxxx>