Date:   Fri, 28 Apr 2017 12:11:02 +0800
From:   Shrirang Bagul <shrirang.bagul@...onical.com>
To:     Jonathan Cameron <jic23@...nel.org>
Cc:     linux-iio@...r.kernel.org, linus.walleij@...aro.org,
        lorenzo.bianconi83@...il.com, gregor.boirie@...rot.com,
        rob@...nel.org, linux-kernel@...r.kernel.org,
        m.niestroj@...nn-global.com
Subject: Re: [PATCH] iio: st_pressure: st_accel: Initialise sensor platform
 data properly

On Wed, 2017-04-26 at 06:37 +0100, Jonathan Cameron wrote:
> On 19/04/17 15:05, Shrirang Bagul wrote:
> > This patch fixes the sensor platform data initialisation for st_pressure
> > and st_accel device drivers. Without this patch, the driver fails to
> > register the sensors when the user removes and re-loads the driver.
> > 
> > 1. Unload the kernel modules for st_pressure
> > $ sudo rmmod st_pressure_i2c
> > $ sudo rmmod st_pressure
> > 
> > 2. Re-load the driver
> > $ sudo insmod st_pressure
> > $ sudo insmod st_pressure_i2c
> > --- OR ---
> > $ sudo modprobe st_pressure_i2c
> > 
> > dmesg errors:
> > 
> > [ 160.935707] iio iio:device2: DRDY on pdata not valid.
> > [ 160.941505] st-press-i2c: probe of i2c-SMO9210:00 failed with error -22
> > 
> > The driver fails to register the pressure sensor device. Devices
> > supported by st_accel driver also suffer from the same bug.
> > 
> > Signed-off-by: Shrirang Bagul <shrirang.bagul@...onical.com>
> 
> Unless I am missing something, the severity of this bug is fairly minor
> so whilst I'm pleased to have it fixed, I'm not intending to mark this
> for stable.
> 
> If anyone has a cunning way of exploiting it then let me know!
Stress testing st_pressure_i2c, st_pressure module load/unload does cause the kernel
to panic.

Apr 27 18:11:21 caracalla kernel: [ 8417.756968] BUG: unable to handle kernel paging
request at ffffffffc0377c48
Apr 27 18:11:21 caracalla kernel: [ 8417.764869] IP: [<ffffffffc03319a6>]
st_sensors_init_sensor+0x26/0x1f0 [st_sensors]
Apr 27 18:11:21 caracalla kernel: [ 8417.773550] PGD 2e0d067 PUD 2e0f067 PMD 3792b067
PTE 0
Apr 27 18:11:21 caracalla kernel: [ 8417.779382] Oops: 0000 [#1] SMP
Apr 27 18:11:21 caracalla kernel: [ 8417.783045] Modules linked in:
st_pressure_i2c(+) st_pressure hts221_i2c msr cmac algif_hash algif_skcipher af_alg
hci_vhci rfcomm bnep arc4 iTCO_wdt iTCO_vendor_support snd_soc_sst_bytcr_rt5660
dell_wmi sparse_keymap ven_rsi_sdio ven_rsi_91x intel_soc_dts_iosf bluetooth
intel_powerclamp coretemp kvm_intel kvm dcdbas mac80211 irqbypass punit_atom_debug
crct10dif_pclmul crc32_pclmul ghash_clmulni_intel aesni_intel aes_x86_64 cfg80211 lrw
gf128mul glue_helper ablk_helper input_leds cryptd psmouse serio_raw cdc_mbim cdc_wdm
cdc_ncm uas r8169 usbnet cdc_acm lpc_ich mii mei_txe mei snd_intel_sst_acpi shpchp
snd_intel_sst_core snd_soc_rt5660 snd_soc_sst_mfld_platform snd_soc_rl6231
snd_soc_core 8250_fintek fjes st_accel_i2c st_accel st_sensors_i2c hts221 i2c_hid
st_sensors snd_compress ad5593r ac97_bus hid industrialio_triggered_buffer kfifo_buf
ad5592r_base industrialio snd_pcm_dmaengine snd_pcm tpm_crb snd_timer dw_dmac snd
dw_dmac_core soundcore rfkill_gpio mac_hid spi_pxa2xx_platform
i2c_designware_platform i2c_designware_core pwm_lpss_platform snd_soc_sst_acpi
8250_dw pwm_lpss iptable_filter ip_tables ip6table_filter ip6_tables x_tables autofs4
sdhci_pci virtio_scsi ahci libahci mmc_block i915 i2c_algo_bit drm_kms_helper
syscopyarea sysfillrect sysimgblt fb_sys_fops usb_storage drm wmi video sdhci_acpi
sdhci [last unloaded: st_pressure]
Apr 27 18:11:21 caracalla kernel: [ 8417.917926] CPU: 0 PID: 15523 Comm: modprobe Not
tainted 4.4.0-77-generic #98-Ubuntu
Apr 27 18:11:21 caracalla kernel: [ 8417.926686] Hardware name: Dell Inc. Edge
Gateway 3003/ , BIOS 00.00.28 03/23/2017
Apr 27 18:11:21 caracalla kernel: [ 8417.935739] task: ffff88007521a640 ti:
ffff88007639c000 task.ti: ffff88007639c000
Apr 27 18:11:21 caracalla kernel: [ 8417.944202] RIP: 0010:[<ffffffffc03319a6>]
[<ffffffffc03319a6>] st_sensors_init_sensor+0x26/0x1f0 [st_sensors]
Apr 27 18:11:21 caracalla kernel: [ 8417.955627] RSP: 0018:ffff88007639fac8 EFLAGS:
00010206
Apr 27 18:11:21 caracalla kernel: [ 8417.961632] RAX: ffffffffc0328640 RBX:
ffff880076832800 RCX: 0000000000000003
Apr 27 18:11:21 caracalla kernel: [ 8417.969702] RDX: ffff8800371c9820 RSI:
ffffffffc0377c48 RDI: ffff880076832800
Apr 27 18:11:21 caracalla kernel: [ 8417.977772] RBP: ffff88007639fad0 R08:
ffff88007639c000 R09: 0000000000000000
Apr 27 18:11:21 caracalla kernel: [ 8417.985841] R10: 00000000000000b6 R11:
0000000000000000 R12: 0000000000000000
Apr 27 18:11:21 caracalla kernel: [ 8417.993911] R13: 0000000000000003 R14:
ffff880076832cc0 R15: ffffffffc03730a0
Apr 27 18:11:21 caracalla kernel: [ 8418.001982] FS: 00007f71faa83700(0000)
GS:ffff880070a00000(0000) knlGS:0000000000000000
Apr 27 18:11:21 caracalla kernel: [ 8418.011133] CS: 0010 DS: 0000 ES: 0000 CR0:
0000000080050033
Apr 27 18:11:21 caracalla kernel: [ 8418.017629] CR2: ffffffffc0377c48 CR3:
0000000074d7d000 CR4: 00000000001006f0
Apr 27 18:11:21 caracalla kernel: [ 8418.025698] Stack:
Apr 27 18:11:21 caracalla kernel: [ 8418.027967] ffff880076832800 ffff88007639faf8
ffffffffc03270ea ffff880076832800
Apr 27 18:11:21 caracalla kernel: [ 8418.036363] ffff8800371c9800 ffff8800371c9820
ffff88007639fb30 ffffffffc037207d
Apr 27 18:11:21 caracalla kernel: [ 8418.044757] ffffffffc03730a0 ffff8800371c9820
ffff8800371c9800 ffffffffc0372020
Apr 27 18:11:21 caracalla kernel: [ 8418.053152] Call Trace:
Apr 27 18:11:21 caracalla kernel: [ 8418.055920] [<ffffffffc03270ea>]
st_press_common_probe+0xea/0x1a0 [st_pressure]
Apr 27 18:11:21 caracalla kernel: [ 8418.064292] [<ffffffffc037207d>]
st_press_i2c_probe+0x5d/0xdc [st_pressure_i2c]
Apr 27 18:11:21 caracalla kernel: [ 8418.072660] [<ffffffffc0372020>] ?
st_press_i2c_remove+0x20/0x20 [st_pressure_i2c]
Apr 27 18:11:21 caracalla kernel: [ 8418.081325] [<ffffffff8168bff0>]
i2c_device_probe+0x100/0x1b0
Apr 27 18:11:21 caracalla kernel: [ 8418.087925] [<ffffffff8155d962>]
driver_probe_device+0x222/0x4a0
Apr 27 18:11:21 caracalla kernel: [ 8418.094819] [<ffffffff8155dc64>]
__driver_attach+0x84/0x90
Apr 27 18:11:21 caracalla kernel: [ 8418.101122] [<ffffffff8155dbe0>] ?
driver_probe_device+0x4a0/0x4a0
Apr 27 18:11:21 caracalla kernel: [ 8418.108212] [<ffffffff8155b58c>]
bus_for_each_dev+0x6c/0xc0
Apr 27 18:11:21 caracalla kernel: [ 8418.114614] [<ffffffff8155d11e>]
driver_attach+0x1e/0x20
Apr 27 18:11:21 caracalla kernel: [ 8418.120720] [<ffffffff8155cc5b>]
bus_add_driver+0x1eb/0x280
Apr 27 18:11:21 caracalla kernel: [ 8418.127121] [<ffffffffc00c5000>] ?
0xffffffffc00c5000
Apr 27 18:11:21 caracalla kernel: [ 8418.132933] [<ffffffff8155e570>]
driver_register+0x60/0xe0
Apr 27 18:11:21 caracalla kernel: [ 8418.143014] [<ffffffff8168cbb1>]
i2c_register_driver+0x41/0xa0
Apr 27 18:11:21 caracalla kernel: [ 8418.153492] [<ffffffffc00c5017>]
st_press_driver_init+0x17/0x1000 [st_pressure_i2c]
Apr 27 18:11:21 caracalla kernel: [ 8418.166063] [<ffffffff81002123>]
do_one_initcall+0xb3/0x200
Apr 27 18:11:21 caracalla kernel: [ 8418.176244] [<ffffffff81837a48>] ?
preempt_schedule_common+0x18/0x30
Apr 27 18:11:21 caracalla kernel: [ 8418.187259] [<ffffffff81837a7c>] ?
_cond_resched+0x1c/0x30
Apr 27 18:11:21 caracalla kernel: [ 8418.197250] [<ffffffff811ed1a3>] ?
kmem_cache_alloc_trace+0x183/0x1f0
Apr 27 18:11:21 caracalla kernel: [ 8418.208323] [<ffffffff8118da83>]
do_init_module+0x5f/0x1cf
Apr 27 18:11:21 caracalla kernel: [ 8418.218242] [<ffffffff8110a9af>]
load_module+0x166f/0x1c10
Apr 27 18:11:21 caracalla kernel: [ 8418.228065] [<ffffffff81106f50>] ?
__symbol_put+0x60/0x60
Apr 27 18:11:21 caracalla kernel: [ 8418.237698] [<ffffffff81214e20>] ?
kernel_read+0x50/0x80
Apr 27 18:11:21 caracalla kernel: [ 8418.247133] [<ffffffff8110b194>]
SYSC_finit_module+0xb4/0xe0
Apr 27 18:11:21 caracalla kernel: [ 8418.256869] [<ffffffff8110b1de>]
SyS_finit_module+0xe/0x10
Apr 27 18:11:21 caracalla kernel: [ 8418.266311] [<ffffffff8183b972>]
entry_SYSCALL_64_fastpath+0x16/0x71
Apr 27 18:11:21 caracalla kernel: [ 8418.276651] Code: 0f 1f 44 00 00 0f 1f 44 00 00
55 48 85 f6 48 89 e5 53 48 89 fb 74 57 48 8b 87 d0 04 00 00 80 b8 8e 01 00 00 00 0f
84 04 01 00 00 <0f> b6 16 80 fa 01 0f 84 30 01 00 00 80 fa 02 0f 85 0f 01 00 00
Apr 27 18:11:21 caracalla kernel: [ 8418.304581] RIP [<ffffffffc03319a6>]
st_sensors_init_sensor+0x26/0x1f0 [st_sensors]
Apr 27 18:11:21 caracalla kernel: [ 8418.316345] RSP <ffff88007639fac8>
Apr 27 18:11:21 caracalla kernel: [ 8418.323222] CR2: ffffffffc0377c48
Apr 27 18:11:21 caracalla kernel: [ 8418.336671] ---[ end trace 7d11c35270e28483 ]---

Thanks,
Shrirang
> 
> Applied to the togreg branch of iio.git and pushed out as testing for the
> autobuilders to play with it.
> 
> Thanks,
> 
> Jonathan
> > ---
> >  drivers/iio/accel/st_accel_core.c       | 7 ++++---
> >  drivers/iio/pressure/st_pressure_core.c | 8 ++++----
> >  2 files changed, 8 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/iio/accel/st_accel_core.c
> > b/drivers/iio/accel/st_accel_core.c
> > index 784670e2736b..07d1489cd457 100644
> > --- a/drivers/iio/accel/st_accel_core.c
> > +++ b/drivers/iio/accel/st_accel_core.c
> > @@ -710,6 +710,8 @@ static const struct iio_trigger_ops st_accel_trigger_ops = {
> >  int st_accel_common_probe(struct iio_dev *indio_dev)
> >  {
> >  	struct st_sensor_data *adata = iio_priv(indio_dev);
> > +	struct st_sensors_platform_data *pdata =
> > +		(struct st_sensors_platform_data *)adata->dev->platform_data;
> >  	int irq = adata->get_irq_data_ready(indio_dev);
> >  	int err;
> >  
> > @@ -736,9 +738,8 @@ int st_accel_common_probe(struct iio_dev *indio_dev)
> >  					&adata->sensor_settings->fs.fs_avl[0];
> >  	adata->odr = adata->sensor_settings->odr.odr_avl[0].hz;
> >  
> > -	if (!adata->dev->platform_data)
> > -		adata->dev->platform_data =
> > -			(struct st_sensors_platform_data *)&default_accel_pdata;
> > +	if (!pdata)
> > +		pdata = (struct st_sensors_platform_data *)&default_accel_pdata;
> >  
> >  	err = st_sensors_init_sensor(indio_dev, adata->dev->platform_data);
> >  	if (err < 0)
> > diff --git a/drivers/iio/pressure/st_pressure_core.c
> > b/drivers/iio/pressure/st_pressure_core.c
> > index 5f2680855552..0d8d5665769a 100644
> > --- a/drivers/iio/pressure/st_pressure_core.c
> > +++ b/drivers/iio/pressure/st_pressure_core.c
> > @@ -567,6 +567,8 @@ static const struct iio_trigger_ops st_press_trigger_ops = {
> >  int st_press_common_probe(struct iio_dev *indio_dev)
> >  {
> >  	struct st_sensor_data *press_data = iio_priv(indio_dev);
> > +	struct st_sensors_platform_data *pdata =
> > +		(struct st_sensors_platform_data *)press_data->dev-
> > >platform_data;
> >  	int irq = press_data->get_irq_data_ready(indio_dev);
> >  	int err;
> >  
> > @@ -602,10 +604,8 @@ int st_press_common_probe(struct iio_dev *indio_dev)
> >  	press_data->odr = press_data->sensor_settings->odr.odr_avl[0].hz;
> >  
> >  	/* Some devices don't support a data ready pin. */
> > -	if (!press_data->dev->platform_data &&
> > -				press_data->sensor_settings->drdy_irq.addr)
> > -		press_data->dev->platform_data =
> > -			(struct st_sensors_platform_data *)&default_press_pdata;
> > +	if (!pdata && press_data->sensor_settings->drdy_irq.addr)
> > +		pdata =	(struct st_sensors_platform_data
> > *)&default_press_pdata;
> >  
> >  	err = st_sensors_init_sensor(indio_dev, press_data->dev->platform_data);
> >  	if (err < 0)
> > 
> 
> 

Powered by blists - more mailing lists