[PATCH] media: atomisp: fix NULL pointer dereference

[PATCH] media: atomisp: fix NULL pointer dereference

[Jiri Slaby]

I am currently seeing:
BUG: kernel NULL pointer dereference, address: 0000000000000002
...
Hardware name: UMAX VisionBook 10Wi Pro/CQM1018CWP, BIOS CQ1018.007 09/22/2016
RIP: 0010:gmin_subdev_add.cold+0x303/0x312 [atomisp_gmin_platform]
...
Call Trace:
 gmin_camera_platform_data+0x2f/0x60 [atomisp_gmin_platform]
 ov2680_probe+0x7f/0x2b0 [atomisp_ov2680]
 i2c_device_probe+0x95/0x290

power can be NULL and that is properly handled earlier in this function.
Even i2c address is set there. So this is a duplicated assignment which
can cause the bug above. Remove it.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c b/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
index 1af9da8acf4c..246742f44d84 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
@@ -574,7 +574,6 @@ static struct gmin_subdev *gmin_subdev_add(struct v4l2_subdev *subdev)
 		gmin_subdevs[i].eldo2_ctrl_shift = gmin_get_var_int(dev, false,
 								    "eldo2_ctrl_shift",
 								    ELDO2_CTRL_SHIFT);
-		gmin_subdevs[i].pwm_i2c_addr = power->addr;
 		break;
 
 	default:
-- 
2.27.0

Re: [PATCH] media: atomisp: fix NULL pointer dereference

[Jiri Slaby]

On 16. 07. 20, 13:51, Jiri Slaby wrote:
> I am currently seeing:
> BUG: kernel NULL pointer dereference, address: 0000000000000002
> ...
> Hardware name: UMAX VisionBook 10Wi Pro/CQM1018CWP, BIOS CQ1018.007 09/22/2016
> RIP: 0010:gmin_subdev_add.cold+0x303/0x312 [atomisp_gmin_platform]
> ...
> Call Trace:
>  gmin_camera_platform_data+0x2f/0x60 [atomisp_gmin_platform]
>  ov2680_probe+0x7f/0x2b0 [atomisp_ov2680]
>  i2c_device_probe+0x95/0x290
> 
> power can be NULL and that is properly handled earlier in this function.
> Even i2c address is set there. So this is a duplicated assignment which
> can cause the bug above. Remove it.

BTW, the camera still doesn't work, but the kernel no longer crashes:

> atomisp_ov2680: module is from the staging directory, the quality is unknown, you have been warned.
> ov2680 i2c-OVTI2680:00: gmin_subdev_add: ACPI detected it on bus ID=CAMB, HID=OVTI2680
> ov2680 i2c-OVTI2680:00: found 'INT33F4:00' at address 0x34, adapter 6
> ov2680 i2c-OVTI2680:00: gmin: power management provided via XPower AXP288 PMIC (i2c addr 0x34)

For this CAM, the address is likely OK.

> ov2680 i2c-OVTI2680:00: found _DSM entry for 'CamClk': 1
> ov2680 i2c-OVTI2680:00: didn't found _DSM entry for 'ClkSrc'
> ov2680 i2c-OVTI2680:00: Failed to find EFI variable OVTI2680:00_ClkSrc
> ov2680 i2c-OVTI2680:00: ClkSrc: using default (1)
> ov2680 i2c-OVTI2680:00: found _DSM entry for 'CsiPort': 0
> ov2680 i2c-OVTI2680:00: found _DSM entry for 'CsiLanes': 2
> ov2680 i2c-OVTI2680:00: didn't found _DSM entry for 'eldo1_1p8v'
> ov2680 i2c-OVTI2680:00: Failed to find EFI variable OVTI2680:00_eldo1_1p8v
> ov2680 i2c-OVTI2680:00: eldo1_1p8v: using default (22)
> ov2680 i2c-OVTI2680:00: didn't found _DSM entry for 'eldo1_sel_reg'
> ov2680 i2c-OVTI2680:00: Failed to find EFI variable OVTI2680:00_eldo1_sel_reg
> ov2680 i2c-OVTI2680:00: eldo1_sel_reg: using default (25)
> ov2680 i2c-OVTI2680:00: didn't found _DSM entry for 'eldo1_ctrl_shift'
> ov2680 i2c-OVTI2680:00: Failed to find EFI variable OVTI2680:00_eldo1_ctrl_shift
> ov2680 i2c-OVTI2680:00: eldo1_ctrl_shift: using default (0)
> ov2680 i2c-OVTI2680:00: didn't found _DSM entry for 'eldo2_1p8v'
> ov2680 i2c-OVTI2680:00: Failed to find EFI variable OVTI2680:00_eldo2_1p8v
> ov2680 i2c-OVTI2680:00: eldo2_1p8v: using default (22)
> ov2680 i2c-OVTI2680:00: didn't found _DSM entry for 'eldo2_sel_reg'
> ov2680 i2c-OVTI2680:00: Failed to find EFI variable OVTI2680:00_eldo2_sel_reg
> ov2680 i2c-OVTI2680:00: eldo2_sel_reg: using default (26)
> ov2680 i2c-OVTI2680:00: didn't found _DSM entry for 'eldo2_ctrl_shift'
> ov2680 i2c-OVTI2680:00: Failed to find EFI variable OVTI2680:00_eldo2_ctrl_shift
> ov2680 i2c-OVTI2680:00: eldo2_ctrl_shift: using default (1)
> ov2680 i2c-OVTI2680:00: power_ctrl: off
> ov2680 i2c-OVTI2680:00: Failed to find EFI gmin variable gmin_V1P8GPIO
> ov2680 i2c-OVTI2680:00: V1P8GPIO: using default (-1)
> ov2680 i2c-OVTI2680:00: Failed to find EFI gmin variable gmin_V2P8GPIO
> ov2680 i2c-OVTI2680:00: V2P8GPIO: using default (-1)
> ov2680 i2c-OVTI2680:00: power_ctrl: on
> ov2680 i2c-OVTI2680:00: I2C write, addr: 0x34, reg: 0x1a, value: 0x16, mask: 0xff
> ov2680 i2c-OVTI2680:00: I2C write, addr: 0x34, reg: 0x1a, value: 0x02, mask: 0x02
> ov2680 i2c-OVTI2680:00: I2C write, addr: 0x34, reg: 0x19, value: 0x16, mask: 0xff
> ov2680 i2c-OVTI2680:00: I2C write, addr: 0x34, reg: 0x19, value: 0x01, mask: 0x01
> ov2680 i2c-OVTI2680:00: I2C write, addr: 0x34, reg: 0x1a, value: 0x16, mask: 0xff
> ov2680 i2c-OVTI2680:00: I2C write, addr: 0x34, reg: 0x1a, value: 0x00, mask: 0x02
> ov2680 i2c-OVTI2680:00: I2C write, addr: 0x34, reg: 0x28, value: 0x16, mask: 0xff
> ov2680 i2c-OVTI2680:00: I2C write, addr: 0x34, reg: 0x28, value: 0x20, mask: 0x20
> ov2680 i2c-OVTI2680:00: camera pdata: port: 0 lanes: 2 order: 00000002
> ov2680 i2c-OVTI2680:00: read error: reg=0x300a: -121

EREMOTEIO. So it shomehow doesn't work.

> ov2680 i2c-OVTI2680:00: sensor_id_high = 0x2
> ov2680 i2c-OVTI2680:00: ov2680_detect err s_config.
> ov2680 i2c-OVTI2680:00: power_ctrl: off
> ov2680 i2c-OVTI2680:00: I2C write, addr: 0x34, reg: 0x19, value: 0x16, mask: 0xff
> ov2680 i2c-OVTI2680:00: I2C write, addr: 0x34, reg: 0x19, value: 0x00, mask: 0x01
> ov2680 i2c-OVTI2680:00: I2C write, addr: 0x34, reg: 0x1a, value: 0x16, mask: 0xff
> ov2680 i2c-OVTI2680:00: I2C write, addr: 0x34, reg: 0x1a, value: 0x00, mask: 0x02
> ov2680 i2c-OVTI2680:00: I2C write, addr: 0x34, reg: 0x28, value: 0x16, mask: 0xff
> ov2680 i2c-OVTI2680:00: I2C write, addr: 0x34, reg: 0x28, value: 0x00, mask: 0x20
> ov2680 i2c-OVTI2680:00: sensor power-gating failed
> ov2680 i2c-OVTI2680:00: +++ out free

Another CAM:

> ov2680 i2c-OVTI2680:01: gmin_subdev_add: ACPI detected it on bus ID=CAMC, HID=OVTI2680
> ov2680 i2c-OVTI2680:01: gmin: power management provided via XPower AXP288 PMIC

now pmic_id is non-zero, so power is not initalized and causes the i2c
address below to be zero.

So either power should be static in that function or pmic_id should be
non-global (per device).

> ov2680 i2c-OVTI2680:01: found _DSM entry for 'CamClk': 1
> ov2680 i2c-OVTI2680:01: didn't found _DSM entry for 'ClkSrc'
> ov2680 i2c-OVTI2680:01: Failed to find EFI variable OVTI2680:01_ClkSrc
> ov2680 i2c-OVTI2680:01: ClkSrc: using default (1)
> ov2680 i2c-OVTI2680:01: found _DSM entry for 'CsiPort': 1
> ov2680 i2c-OVTI2680:01: found _DSM entry for 'CsiLanes': 1
> ov2680 i2c-OVTI2680:01: didn't found _DSM entry for 'eldo1_1p8v'
> ov2680 i2c-OVTI2680:01: Failed to find EFI variable OVTI2680:01_eldo1_1p8v
> ov2680 i2c-OVTI2680:01: eldo1_1p8v: using default (22)
> ov2680 i2c-OVTI2680:01: didn't found _DSM entry for 'eldo1_sel_reg'
> ov2680 i2c-OVTI2680:01: Failed to find EFI variable OVTI2680:01_eldo1_sel_reg
> ov2680 i2c-OVTI2680:01: eldo1_sel_reg: using default (25)
> ov2680 i2c-OVTI2680:01: didn't found _DSM entry for 'eldo1_ctrl_shift'
> ov2680 i2c-OVTI2680:01: Failed to find EFI variable OVTI2680:01_eldo1_ctrl_shift
> ov2680 i2c-OVTI2680:01: eldo1_ctrl_shift: using default (0)
> ov2680 i2c-OVTI2680:01: didn't found _DSM entry for 'eldo2_1p8v'
> ov2680 i2c-OVTI2680:01: Failed to find EFI variable OVTI2680:01_eldo2_1p8v
> ov2680 i2c-OVTI2680:01: eldo2_1p8v: using default (22)
> ov2680 i2c-OVTI2680:01: didn't found _DSM entry for 'eldo2_sel_reg'
> ov2680 i2c-OVTI2680:01: Failed to find EFI variable OVTI2680:01_eldo2_sel_reg
> ov2680 i2c-OVTI2680:01: eldo2_sel_reg: using default (26)
> ov2680 i2c-OVTI2680:01: didn't found _DSM entry for 'eldo2_ctrl_shift'
> ov2680 i2c-OVTI2680:01: Failed to find EFI variable OVTI2680:01_eldo2_ctrl_shift
> ov2680 i2c-OVTI2680:01: eldo2_ctrl_shift: using default (1)
> ov2680 i2c-OVTI2680:01: power_ctrl: off
> ov2680 i2c-OVTI2680:01: power_ctrl: on
> ov2680 i2c-OVTI2680:01: I2C write, addr: 0x00, reg: 0x1a, value: 0x16, mask: 0xff
> intel_soc_pmic_exec_mipi_pmic_seq_element: Unexpected i2c-addr: 0x00 (reg-addr 0x1a value 0x16 mask 0xff)
> ov2680 i2c-OVTI2680:01: I2C write, addr: 0x00, reg: 0x28, value: 0x16, mask: 0xff
> intel_soc_pmic_exec_mipi_pmic_seq_element: Unexpected i2c-addr: 0x00 (reg-addr 0x28 value 0x16 mask 0xff)
> ov2680 i2c-OVTI2680:01: I2C write, addr: 0x00, reg: 0x19, value: 0x16, mask: 0xff
> intel_soc_pmic_exec_mipi_pmic_seq_element: Unexpected i2c-addr: 0x00 (reg-addr 0x19 value 0x16 mask 0xff)
> ov2680 i2c-OVTI2680:01: I2C write, addr: 0x00, reg: 0x28, value: 0x16, mask: 0xff
> intel_soc_pmic_exec_mipi_pmic_seq_element: Unexpected i2c-addr: 0x00 (reg-addr 0x28 value 0x16 mask 0xff)
> ov2680 i2c-OVTI2680:01: power_ctrl: off
> ov2680 i2c-OVTI2680:01: sensor power-up failed
> ov2680 i2c-OVTI2680:01: ov2680 power-up err.
> ov2680 i2c-OVTI2680:01: power_ctrl: off
> ov2680 i2c-OVTI2680:01: sensor power-gating failed
> ov2680 i2c-OVTI2680:01: +++ out free

thanks,
-- 
js
suse labs

Re: [PATCH] media: atomisp: fix NULL pointer dereference

[Andy Shevchenko]

On Thu, Jul 16, 2020 at 2:52 PM Jiri Slaby <jslaby@suse.cz> wrote:
>
> I am currently seeing:
> BUG: kernel NULL pointer dereference, address: 0000000000000002
> ...
> Hardware name: UMAX VisionBook 10Wi Pro/CQM1018CWP, BIOS CQ1018.007 09/22/2016
> RIP: 0010:gmin_subdev_add.cold+0x303/0x312 [atomisp_gmin_platform]
> ...
> Call Trace:
>  gmin_camera_platform_data+0x2f/0x60 [atomisp_gmin_platform]
>  ov2680_probe+0x7f/0x2b0 [atomisp_ov2680]
>  i2c_device_probe+0x95/0x290
>
> power can be NULL and that is properly handled earlier in this function.
> Even i2c address is set there. So this is a duplicated assignment which
> can cause the bug above. Remove it.

I believe it's fixed in [1].

[1]: https://git.linuxtv.org/mchehab/experimental.git/log/?h=atomisp_v5

> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> Cc: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> ---
>  drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c b/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
> index 1af9da8acf4c..246742f44d84 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
> @@ -574,7 +574,6 @@ static struct gmin_subdev *gmin_subdev_add(struct v4l2_subdev *subdev)
>                 gmin_subdevs[i].eldo2_ctrl_shift = gmin_get_var_int(dev, false,
>                                                                     "eldo2_ctrl_shift",
>                                                                     ELDO2_CTRL_SHIFT);
> -               gmin_subdevs[i].pwm_i2c_addr = power->addr;
>                 break;
>
>         default:
> --
> 2.27.0
>


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] media: atomisp: fix NULL pointer dereference

[Jiri Slaby]

On 17. 07. 20, 11:01, Andy Shevchenko wrote:
> On Thu, Jul 16, 2020 at 2:52 PM Jiri Slaby <jslaby@suse.cz> wrote:
>>
>> I am currently seeing:
>> BUG: kernel NULL pointer dereference, address: 0000000000000002
>> ...
>> Hardware name: UMAX VisionBook 10Wi Pro/CQM1018CWP, BIOS CQ1018.007 09/22/2016
>> RIP: 0010:gmin_subdev_add.cold+0x303/0x312 [atomisp_gmin_platform]
>> ...
>> Call Trace:
>>  gmin_camera_platform_data+0x2f/0x60 [atomisp_gmin_platform]
>>  ov2680_probe+0x7f/0x2b0 [atomisp_ov2680]
>>  i2c_device_probe+0x95/0x290
>>
>> power can be NULL and that is properly handled earlier in this function.
>> Even i2c address is set there. So this is a duplicated assignment which
>> can cause the bug above. Remove it.
> 
> I believe it's fixed in [1].
> 
> [1]: https://git.linuxtv.org/mchehab/experimental.git/log/?h=atomisp_v5

It seems so. By:
commit 219448c9cd4a505b6274d746ca1897af20e6d06a
Author: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Date:   Fri Jun 26 14:19:21 2020 +0200

    media: atomisp: Make pointer to PMIC client global

thanks,
-- 
js
suse labs