Re:

Hans Verkuil
2024-08-02T06:45:58Z

On 01/08/2024 15:54, Mikhail Lobanov wrote:
> Subject: [PATCH] cobalt: adding a check to the driver

The subject might be here, but it is not in the actual subject line of the email!

>
> This patch addresses an issue in cobalt-flash.c where the return value of the mtd_device_register function,
> was not being checked. This omission could lead to unhandled errors if the registration fails.
> The patch adds error handling by checking the return value and logging an error message if registration fails.
> It ensures that the function returns the appropriate error code, improving error detection and the robustness
> of the code.
>
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>
> Fixes: 85756a069c55 ("[media] cobalt: add new driver")
> Signed-off-by: Mikhail Lobanov <m.lobanov@rosalinux.ru>
> ---
> drivers/media/pci/cobalt/cobalt-flash.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/pci/cobalt/cobalt-flash.c b/drivers/media/pci/cobalt/cobalt-flash.c
> index 1d3c64b4cf6d..06ad9aaeff1b 100644
> --- a/drivers/media/pci/cobalt/cobalt-flash.c
> +++ b/drivers/media/pci/cobalt/cobalt-flash.c
> @@ -1,4 +1,4 @@
> -// SPDX-License-Identifier: GPL-2.0-only
> +/// SPDX-License-Identifier: GPL-2.0-only

Why?

> /*
> * Cobalt NOR flash functions
> *
> @@ -104,6 +104,10 @@ int cobalt_flash_probe(struct cobalt *cobalt)
> mtd->owner = THIS_MODULE;
> mtd->dev.parent = &cobalt->pci_dev->dev;
> mtd_device_register(mtd, NULL, 0);
> + if (ret) {
> + cobalt_err("Registering MTD device failed with error %d\n", ret);
> + return ret;
> + }

This is obviously wrong, and just as importantly, the caller of cobalt_flash_probe
doesn't check the return code either.

The indentation is wrong as well.

This is a really poor patch...

Regards,

Hans

> return 0;
> }
>