On Thu, Nov 10, 2022 at 04:41:28PM +0000, Dave Stevenson wrote: > On Thu, 10 Nov 2022 at 16:31, coverity-bot <keescook@xxxxxxxxxxxx> wrote: > > > > Hello! > > > > This is an experimental semi-automated report about issues detected by > > Coverity from a scan of next-20221110 as part of the linux-next scan project: > > https://scan.coverity.com/projects/linux-next-weekly-scan > > > > You're getting this email because you were associated with the identified > > lines of code (noted below) that were touched by commits: > > > > Thu Oct 27 14:38:02 2022 +0300 > > 4c9c93cf8657 ("media: i2c: imx290: Create controls for fwnode properties") > > > > Coverity reported the following: > > > > *** CID 1527251: Error handling issues (CHECKED_RETURN) > > drivers/media/i2c/imx290.c:1056 in imx290_ctrl_init() > > 1050 imx290->vblank = v4l2_ctrl_new_std(&imx290->ctrls, &imx290_ctrl_ops, > > 1051 V4L2_CID_VBLANK, blank, blank, 1, > > 1052 blank); > > 1053 if (imx290->vblank) > > 1054 imx290->vblank->flags |= V4L2_CTRL_FLAG_READ_ONLY; > > 1055 > > vvv CID 1527251: Error handling issues (CHECKED_RETURN) > > vvv Calling "v4l2_ctrl_new_fwnode_properties" without checking return value (as is done elsewhere 9 out of 10 times). > > 1056 v4l2_ctrl_new_fwnode_properties(&imx290->ctrls, &imx290_ctrl_ops, > > 1057 &props); > > 1058 > > 1059 imx290->sd.ctrl_handler = &imx290->ctrls; > > 1060 > > 1061 if (imx290->ctrls.error) { > > > > If this is a false positive, please let us know so we can mark it as > > such, or teach the Coverity rules to be smarter. If not, please make > > sure fixes get into linux-next. :) For patches fixing this, please > > include these lines (but double-check the "Fixes" first): > > I looked at this one when the patches were sent to the list. > > On failure, v4l2_ctrl_new_fwnode_properties will have set the error > flag in struct v4l2_ctrl_handler. This is also what it returns. > > In most of the existing drivers the error flag has already been > checked before calling v4l2_ctrl_new_fwnode_properties, therefore the > return value has to be checked explicitly. In this case it is checked > at line 1061 which is after v4l2_ctrl_new_fwnode_properties has been > called, and therefore there is no need to check the return value of > the call. > > IMHO Neither is particularly right or wrong, just slightly different > approaches. In some regards this new code pattern is nicer as it > removes a number of error handling paths. Great! Thanks for double-checking it. :) -- Kees Cook
