lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 10 Aug 2021 18:36:59 +0300 From: Evgeny Novikov <novikov@...ras.ru> To: Dan Carpenter <dan.carpenter@...cle.com> Cc: Mauro Carvalho Chehab <mchehab@...nel.org>, Sakari Ailus <sakari.ailus@...ux.intel.com>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Alex Dewar <alex.dewar90@...il.com>, Colin Ian King <colin.king@...onical.com>, Linus Walleij <linus.walleij@...aro.org>, linux-media@...r.kernel.org, linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org, ldv-project@...uxtesting.org Subject: Re: [PATCH] media: atomisp: Fix error handling in probe Hi Dan, Thank you for these suggestions. I will resend the patch. Best regards, Evgeny Novikov On 10.08.2021 18:14, Dan Carpenter wrote: > On Tue, Aug 10, 2021 at 02:53:03PM +0300, Evgeny Novikov wrote: >> There were several issues with handling errors in lm3554_probe(): >> - Probe did not set the error code when v4l2_ctrl_handler_init() failed. >> - It intermixed gotos for handling errors of v4l2_ctrl_handler_init() >> and media_entity_pads_init(). >> - Probe did not free resources in case of failures of >> atomisp_register_i2c_module(). >> >> The patch fixes all these issues. >> >> Found by Linux Driver Verification project (linuxtesting.org). > This patch is good, but two things: > > 1) Please can you rename all the labels to say what the goto does? > > err_uninit: > lm3554_gpio_uninit(client); > cleanup_media: > media_entity_cleanup(&flash->sd.entity); > free_handler: > v4l2_ctrl_handler_free(&flash->ctrl_handler); > unregister_subdev: > v4l2_device_unregister_subdev(&flash->sd); > > 2) There is a missing error code still: > > drivers/staging/media/atomisp/i2c/atomisp-lm3554.c > 858 if (ret) { > 859 dev_err(&client->dev, "error initialize a ctrl_handler.\n"); > 860 goto fail3; > 861 } > 862 > 863 for (i = 0; i < ARRAY_SIZE(lm3554_controls); i++) > 864 v4l2_ctrl_new_custom(&flash->ctrl_handler, &lm3554_controls[i], > 865 NULL); > 866 > 867 if (flash->ctrl_handler.error) { > 868 dev_err(&client->dev, "ctrl_handler error.\n"); > 869 goto fail3; > ^^^^^^^^^^ > err = flash->ctrl_handler.error; > > 870 } > 871 > 872 flash->sd.ctrl_handler = &flash->ctrl_handler; > 873 err = media_entity_pads_init(&flash->sd.entity, 0, NULL); > 874 if (err) { > 875 dev_err(&client->dev, "error initialize a media entity.\n"); > 876 goto fail2; > 877 } > > regards, > dan carpenter >
Powered by blists - more mailing lists