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:14:27 +0300 From: Dan Carpenter <dan.carpenter@...cle.com> To: Evgeny Novikov <novikov@...ras.ru> 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 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