On 07/22/2011 11:26 AM, Subash Patel wrote: >>> ... >>>> + >>>> +static int fimc_md_register_sensor_entities(struct fimc_md *fmd) >>>> +{ >>>> + struct s5p_platform_fimc *pdata = fmd->pdev->dev.platform_data; >>>> + struct fimc_dev *fd = NULL; >>>> + int num_clients, ret, i; >>>> + >>>> + /* >>>> + * Runtime resume one of the FIMC entities to make sure >>>> + * the sclk_cam clocks are not globally disabled. >>>> + */ >>>> + for (i = 0; !fd&& i< ARRAY_SIZE(fmd->fimc); i++) >>>> + if (fmd->fimc[i]) >>>> + fd = fmd->fimc[i]; >>>> + if (!fd) >>>> + return -ENXIO; >>>> + ret = pm_runtime_get_sync(&fd->pdev->dev); >>>> + if (ret< 0) >>>> + return ret; >>>> + >>>> + WARN_ON(pdata->num_clients> ARRAY_SIZE(fmd->sensor)); >>>> + num_clients = min_t(u32, pdata->num_clients, ARRAY_SIZE(fmd->sensor)); >>>> + >>>> + fmd->num_sensors = num_clients; >>>> + for (i = 0; i< num_clients; i++) { >>>> + fmd->sensor[i].pdata =&pdata->isp_info[i]; >>>> + ret = __fimc_md_set_camclk(fmd,&fmd->sensor[i], true); >>>> + if (ret) >>>> + break; >>>> + fmd->sensor[i].subdev = >>>> + fimc_md_register_sensor(fmd,&fmd->sensor[i]); >>> >>> There is an issue here. Function fimc_md_register_sensor(), >>> can return subdev, as also error codes when i2c_get_adapter() >>> or NULL when v4l2_i2c_new_subdev_board() fail. But we do not >>> invalidate, and assume the return value is valid subdev. It >>> will cause kernel NULL pointer exception later in fimc_md_create_links(). >> >> Thanks for letting know. >> I remember fixing this issue in v2 of the patch set by making >> fimc_md_register_sensor() return NULL on any error, also when >> i2c_get_adapter() fails, rather than ERR_PTR value. >> >> Do you really mean that there is a NULL or _invalid_ pointer >> dereference in fimc_md_create_links() ? >> An oops on a NULL subdev pointer in fmd->sensor[] array seems >> impossible as there is a check at the beginning of the loop: > > If you return NULL, then this check will block a crash. In my case, > I failed to get the i2c_adapter, and ENODEV was returned, which is > not NULL. Hence I pass through this check, and will crash in > > s_info = v4l2_get_subdev_hostdata(sensor); I see, I have also gone through this during the tests and this problem should be fixed already in v3 of the patch-set you've originally replied to. > > I dont have access to your new patch-set. But if you have returned > NULL, then thats should fix this. To make sure you use the most up to date version you could pull from: git://git.infradead.org/users/kmpark/linux-2.6-samsung s5p-fimc-next Or here is a gitweb link: http://git.infradead.org/users/kmpark/linux-2.6-samsung/shortlog/refs/heads/s5p-fimc-next I intend to keep using the "s5p-fimc-next" branch name for any new patches for next kernel version. So in case you get any crashes it might be a good idea to also give the sources there a try. -- Regards, Sylwester -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html