All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] iio: core: fix list breakage in iio_device_unregister()
@ 2021-04-27  2:20 Frank Zago
  2021-04-27  2:20 ` [PATCH 2/2] iio: light: tcs3472: do not free unallocated IRQ Frank Zago
  2021-04-27  8:23 ` [PATCH 1/2] iio: core: fix list breakage in iio_device_unregister() Jonathan Cameron
  0 siblings, 2 replies; 5+ messages in thread
From: Frank Zago @ 2021-04-27  2:20 UTC (permalink / raw)
  To: Jonathan Cameron, Lars-Peter Clausen, Alexandru Ardelean,
	frank zago, linux-iio, linux-kernel

From: frank zago <frank@zago.net>

The ioctl handlers are unlinked and freed in
iio_buffers_free_sysfs_and_mask(). Do not break the list prematurely.

This fixes the following GPF when a client module is removed:

[  157.007865] general protection fault, probably for non-canonical address 0xdead000000000100: 0000 [#1] SMP NOPTI
[  157.007876] CPU: 6 PID: 2353 Comm: rmmod Not tainted 5.12.0+ #29
[  157.007882] ...
[  157.007886] RIP: 0010:iio_device_ioctl_handler_unregister+0xd/0x30 [industrialio]
[  157.007904] Code: 00 48 89 77 08 48 89 3e 48 89 e5 48 89 46 08 48 89 30 5d c3 66 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 8b 47 08 48 8b 17 55 <48> 89 42 08 48 89 10 48 89 e5 48 b8 00 01
 00 00 00 00 ad de 5d 48
[  157.007910] RSP: 0018:ffffa9b840a57d70 EFLAGS: 00010202
[  157.007916] RAX: dead000000000122 RBX: ffff95484e9a8000 RCX: 0000000000000000
[  157.007920] RDX: dead000000000100 RSI: 0000000000000246 RDI: ffff9548489a2360
[  157.007923] RBP: ffffa9b840a57da0 R08: 0000000000000000 R09: ffffa9b840a57d18
[  157.007926] R10: 0000000000000000 R11: ffff95489f3e5180 R12: ffff95484e9a8000
[  157.007929] R13: ffff95484e9a8390 R14: ffff95484e9a8000 R15: 0000000000000000
[  157.007933] FS:  00007fd35bd18540(0000) GS:ffff954b4eb80000(0000) knlGS:0000000000000000
[  157.007937] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  157.007941] CR2: 000055e1e29d4818 CR3: 000000010ae76000 CR4: 00000000003506e0
[  157.007945] Call Trace:
[  157.007950]  ? iio_buffers_free_sysfs_and_mask+0x2a/0xb0 [industrialio]
[  157.007965]  iio_device_unregister+0xba/0xc0 [industrialio]
[  157.007978]  tcs3472_remove+0x1e/0x90 [tcs3472]
[  157.007984]  i2c_device_remove+0x2b/0xa0
[  157.007993]  __device_release_driver+0x181/0x240
[  157.008000]  driver_detach+0xd5/0x120
[  157.008005]  bus_remove_driver+0x5c/0xe0
[  157.008010]  driver_unregister+0x31/0x50
[  157.008014]  i2c_del_driver+0x46/0x70
[  157.008020]  tcs3472_driver_exit+0x10/0x5dd [tcs3472]
[  157.008026]  __do_sys_delete_module.constprop.0+0x183/0x290
[  157.008033]  ? exit_to_user_mode_prepare+0x37/0x1c0
[  157.008041]  __x64_sys_delete_module+0x12/0x20
[  157.008045]  do_syscall_64+0x40/0xb0
[  157.008052]  entry_SYSCALL_64_after_hwframe+0x44/0xae
[  157.008058] RIP: 0033:0x7fd35be5aceb
[  157.008063] Code: 73 01 c3 48 8b 0d 7d 91 0c 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa b8 b0 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 4d 91 0c 00 f7 d8 64 89 01 48
[  157.008068] RSP: 002b:00007ffe90b07478 EFLAGS: 00000206 ORIG_RAX: 00000000000000b0
[  157.008073] RAX: ffffffffffffffda RBX: 000055e1e29ca770 RCX: 00007fd35be5aceb
[  157.008076] RDX: 000000000000000a RSI: 0000000000000800 RDI: 000055e1e29ca7d8
[  157.008079] RBP: 00007ffe90b074d8 R08: 0000000000000000 R09: 0000000000000000
[  157.008081] R10: 00007fd35bed5ac0 R11: 0000000000000206 R12: 00007ffe90b076b0
[  157.008084] R13: 00007ffe90b08807 R14: 000055e1e29ca2a0 R15: 000055e1e29ca770
[  157.008089] Modules linked in: tcs3472(-) industrialio_triggered_buffer kfifo_buf industrialio ch341_buses binfmt_misc snd_hda_codec_realtek snd_hda_codec_generic ledtrig_audio snd_hda_codec_hdmi snd_hda_intel snd_intel_dspcfg snd_hda_codec snd_hwdep snd_hda_core snd_pcm wmi_bmof snd_seq snd_timer snd_seq_device input_leds rapl snd ccp k10temp soundcore wmi mac_hid sch_fq_codel parport_pc ppdev lp parport ip_tables x_tables autofs4 dm_crypt hid_generic usbhid hid radeon i2c_algo_bit drm_ttm_helper ttm drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops cec rc_core drm crct10dif_pclmul crc32_pclmul ghash_clmulni_intel aesni_intel r8169 crypto_simd cryptd ahci i2c_piix4 xhci_pci realtek libahci xhci_pci_renesas gpio_amdpt gpio_generic [last unloaded: tcs3472]
[  157.008167] ---[ end trace c8eef7a3c5a40ff0 ]---

Signed-off-by: frank zago <frank@zago.net>
---
 drivers/iio/industrialio-core.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index d92c58a94fe4..9e59f5da3d28 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -1926,9 +1926,6 @@ EXPORT_SYMBOL(__iio_device_register);
  **/
 void iio_device_unregister(struct iio_dev *indio_dev)
 {
-	struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev);
-	struct iio_ioctl_handler *h, *t;
-
 	cdev_device_del(&indio_dev->chrdev, &indio_dev->dev);
 
 	mutex_lock(&indio_dev->info_exist_lock);
@@ -1939,9 +1936,6 @@ void iio_device_unregister(struct iio_dev *indio_dev)
 
 	indio_dev->info = NULL;
 
-	list_for_each_entry_safe(h, t, &iio_dev_opaque->ioctl_handlers, entry)
-		list_del(&h->entry);
-
 	iio_device_wakeup_eventset(indio_dev);
 	iio_buffer_wakeup_poll(indio_dev);
 
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/2] iio: light: tcs3472: do not free unallocated IRQ
  2021-04-27  2:20 [PATCH 1/2] iio: core: fix list breakage in iio_device_unregister() Frank Zago
@ 2021-04-27  2:20 ` Frank Zago
  2021-04-27 17:23   ` Jonathan Cameron
  2021-04-27  8:23 ` [PATCH 1/2] iio: core: fix list breakage in iio_device_unregister() Jonathan Cameron
  1 sibling, 1 reply; 5+ messages in thread
From: Frank Zago @ 2021-04-27  2:20 UTC (permalink / raw)
  To: Jonathan Cameron, Lars-Peter Clausen, Alexandru Ardelean,
	frank zago, linux-iio, linux-kernel

From: frank zago <frank@zago.net>

Allocating an IRQ is conditional to the IRQ existence, but freeing it
was not. If no IRQ was allocate, the driver would still try to free
IRQ 0. Add the missing checks.

This fixes the following trace when the driver is removed:

[  100.667788] Trying to free already-free IRQ 0
[  100.667793] WARNING: CPU: 0 PID: 2315 at kernel/irq/manage.c:1826 free_irq+0x1fd/0x370
[  100.667804] Modules linked in: tcs3472(-) industrialio_triggered_buffer kfifo_buf industrialio ch341_buses binfmt_misc snd_hda_codec_realtek snd_hda_codec_generic ledtrig_audio snd_hda_codec_hdmi snd_hda_intel snd_intel_dspcfg snd_hda_codec snd_hwdep snd_hda_core wmi_bmof snd_pcm snd_seq rapl input_leds snd_timer snd_seq_device snd k10temp ccp soundcore wmi mac_hid sch_fq_codel parport_pc ppdev lp parport ip_tables x_tables autofs4 dm_crypt hid_generic usbhid hid radeon i2c_algo_bit drm_ttm_helper ttm drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops cec rc_core drm crct10dif_pclmul crc32_pclmul ghash_clmulni_intel aesni_intel crypto_simd r8169 cryptd ahci i2c_piix4 xhci_pci realtek libahci xhci_pci_renesas gpio_amdpt gpio_generic
[  100.667874] CPU: 0 PID: 2315 Comm: rmmod Not tainted 5.12.0+ #29
[  100.667878] ...
[  100.667881] RIP: 0010:free_irq+0x1fd/0x370
[  100.667887] Code: e8 c8 d8 1b 00 48 83 c4 10 4c 89 f8 5b 41 5c 41 5d 41 5e 41 5f 5d c3 8b 75 d0 48 c7 c7 40 8b 36 93 4c 89 4d c8 e8 d1 2c a2 00 <0f> 0b 4c 8b 4d c8 4c 89 f7 4c 89 ce e8 72 c7 a8 00 49 8b 47 40 48
[  100.667891] RSP: 0018:ffff9f44813b7d88 EFLAGS: 00010082
[  100.667895] RAX: 0000000000000000 RBX: ffff8e50caf47800 RCX: ffff8e53cea185c8
[  100.667897] RDX: 00000000ffffffd8 RSI: 0000000000000027 RDI: ffff8e53cea185c0
[  100.667900] RBP: ffff9f44813b7dc0 R08: 0000000000000000 R09: ffff9f44813b7b50
[  100.667902] R10: ffff9f44813b7b48 R11: ffffffff93b53848 R12: ffff8e50c0125080
[  100.667903] R13: ffff8e50c0136f60 R14: ffff8e50c0136ea4 R15: ffff8e50c0136e00
[  100.667906] FS:  00007fa28b899540(0000) GS:ffff8e53cea00000(0000) knlGS:0000000000000000
[  100.667909] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  100.667911] CR2: 0000561851777818 CR3: 000000010633a000 CR4: 00000000003506f0
[  100.667914] Call Trace:
[  100.667920]  tcs3472_remove+0x3a/0x90 [tcs3472]
[  100.667927]  i2c_device_remove+0x2b/0xa0
[  100.667934]  __device_release_driver+0x181/0x240
[  100.667940]  driver_detach+0xd5/0x120
[  100.667944]  bus_remove_driver+0x5c/0xe0
[  100.667947]  driver_unregister+0x31/0x50
[  100.667951]  i2c_del_driver+0x46/0x70
[  100.667955]  tcs3472_driver_exit+0x10/0x5dd [tcs3472]
[  100.667960]  __do_sys_delete_module.constprop.0+0x183/0x290
[  100.667965]  ? exit_to_user_mode_prepare+0x37/0x1c0
[  100.667971]  __x64_sys_delete_module+0x12/0x20
[  100.667974]  do_syscall_64+0x40/0xb0
[  100.667981]  entry_SYSCALL_64_after_hwframe+0x44/0xae
[  100.667985] RIP: 0033:0x7fa28b9dbceb
[  100.667989] Code: 73 01 c3 48 8b 0d 7d 91 0c 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa b8 b0 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 4d 91 0c 00 f7 d8 64 89 01 48
[  100.667992] RSP: 002b:00007ffe02ea7068 EFLAGS: 00000206 ORIG_RAX: 00000000000000b0
[  100.667995] RAX: ffffffffffffffda RBX: 000056185176d750 RCX: 00007fa28b9dbceb
[  100.667997] RDX: 000000000000000a RSI: 0000000000000800 RDI: 000056185176d7b8
[  100.667999] RBP: 00007ffe02ea70c8 R08: 0000000000000000 R09: 0000000000000000
[  100.668001] R10: 00007fa28ba56ac0 R11: 0000000000000206 R12: 00007ffe02ea72a0
[  100.668003] R13: 00007ffe02ea8807 R14: 000056185176d2a0 R15: 000056185176d750
[  100.668007] ---[ end trace b21b0811931d933c ]---

Signed-off-by: frank zago <frank@zago.net>
---
 drivers/iio/light/tcs3472.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/light/tcs3472.c b/drivers/iio/light/tcs3472.c
index a0dc447aeb68..b41068492338 100644
--- a/drivers/iio/light/tcs3472.c
+++ b/drivers/iio/light/tcs3472.c
@@ -531,7 +531,8 @@ static int tcs3472_probe(struct i2c_client *client,
 	return 0;
 
 free_irq:
-	free_irq(client->irq, indio_dev);
+	if (client->irq)
+		free_irq(client->irq, indio_dev);
 buffer_cleanup:
 	iio_triggered_buffer_cleanup(indio_dev);
 	return ret;
@@ -559,7 +560,8 @@ static int tcs3472_remove(struct i2c_client *client)
 	struct iio_dev *indio_dev = i2c_get_clientdata(client);
 
 	iio_device_unregister(indio_dev);
-	free_irq(client->irq, indio_dev);
+	if (client->irq)
+		free_irq(client->irq, indio_dev);
 	iio_triggered_buffer_cleanup(indio_dev);
 	tcs3472_powerdown(iio_priv(indio_dev));
 
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] iio: core: fix list breakage in iio_device_unregister()
  2021-04-27  2:20 [PATCH 1/2] iio: core: fix list breakage in iio_device_unregister() Frank Zago
  2021-04-27  2:20 ` [PATCH 2/2] iio: light: tcs3472: do not free unallocated IRQ Frank Zago
@ 2021-04-27  8:23 ` Jonathan Cameron
  1 sibling, 0 replies; 5+ messages in thread
From: Jonathan Cameron @ 2021-04-27  8:23 UTC (permalink / raw)
  To: Frank Zago
  Cc: Jonathan Cameron, Lars-Peter Clausen, Alexandru Ardelean,
	linux-iio, linux-kernel

On Mon, 26 Apr 2021 21:20:16 -0500
Frank Zago <frank@zago.net> wrote:

> From: frank zago <frank@zago.net>
> 
> The ioctl handlers are unlinked and freed in
> iio_buffers_free_sysfs_and_mask(). Do not break the list prematurely.
> 
> This fixes the following GPF when a client module is removed:
> 
> [  157.007865] general protection fault, probably for non-canonical address 0xdead000000000100: 0000 [#1] SMP NOPTI
> [  157.007876] CPU: 6 PID: 2353 Comm: rmmod Not tainted 5.12.0+ #29
> [  157.007882] ...
> [  157.007886] RIP: 0010:iio_device_ioctl_handler_unregister+0xd/0x30 [industrialio]
> [  157.007904] Code: 00 48 89 77 08 48 89 3e 48 89 e5 48 89 46 08 48 89 30 5d c3 66 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 8b 47 08 48 8b 17 55 <48> 89 42 08 48 89 10 48 89 e5 48 b8 00 01
>  00 00 00 00 ad de 5d 48
> [  157.007910] RSP: 0018:ffffa9b840a57d70 EFLAGS: 00010202
> [  157.007916] RAX: dead000000000122 RBX: ffff95484e9a8000 RCX: 0000000000000000
> [  157.007920] RDX: dead000000000100 RSI: 0000000000000246 RDI: ffff9548489a2360
> [  157.007923] RBP: ffffa9b840a57da0 R08: 0000000000000000 R09: ffffa9b840a57d18
> [  157.007926] R10: 0000000000000000 R11: ffff95489f3e5180 R12: ffff95484e9a8000
> [  157.007929] R13: ffff95484e9a8390 R14: ffff95484e9a8000 R15: 0000000000000000
> [  157.007933] FS:  00007fd35bd18540(0000) GS:ffff954b4eb80000(0000) knlGS:0000000000000000
> [  157.007937] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [  157.007941] CR2: 000055e1e29d4818 CR3: 000000010ae76000 CR4: 00000000003506e0
> [  157.007945] Call Trace:
> [  157.007950]  ? iio_buffers_free_sysfs_and_mask+0x2a/0xb0 [industrialio]
> [  157.007965]  iio_device_unregister+0xba/0xc0 [industrialio]
> [  157.007978]  tcs3472_remove+0x1e/0x90 [tcs3472]
> [  157.007984]  i2c_device_remove+0x2b/0xa0
> [  157.007993]  __device_release_driver+0x181/0x240
> [  157.008000]  driver_detach+0xd5/0x120
> [  157.008005]  bus_remove_driver+0x5c/0xe0
> [  157.008010]  driver_unregister+0x31/0x50
> [  157.008014]  i2c_del_driver+0x46/0x70
> [  157.008020]  tcs3472_driver_exit+0x10/0x5dd [tcs3472]
> [  157.008026]  __do_sys_delete_module.constprop.0+0x183/0x290
> [  157.008033]  ? exit_to_user_mode_prepare+0x37/0x1c0
> [  157.008041]  __x64_sys_delete_module+0x12/0x20
> [  157.008045]  do_syscall_64+0x40/0xb0
> [  157.008052]  entry_SYSCALL_64_after_hwframe+0x44/0xae
> [  157.008058] RIP: 0033:0x7fd35be5aceb
> [  157.008063] Code: 73 01 c3 48 8b 0d 7d 91 0c 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa b8 b0 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 4d 91 0c 00 f7 d8 64 89 01 48
> [  157.008068] RSP: 002b:00007ffe90b07478 EFLAGS: 00000206 ORIG_RAX: 00000000000000b0
> [  157.008073] RAX: ffffffffffffffda RBX: 000055e1e29ca770 RCX: 00007fd35be5aceb
> [  157.008076] RDX: 000000000000000a RSI: 0000000000000800 RDI: 000055e1e29ca7d8
> [  157.008079] RBP: 00007ffe90b074d8 R08: 0000000000000000 R09: 0000000000000000
> [  157.008081] R10: 00007fd35bed5ac0 R11: 0000000000000206 R12: 00007ffe90b076b0
> [  157.008084] R13: 00007ffe90b08807 R14: 000055e1e29ca2a0 R15: 000055e1e29ca770
> [  157.008089] Modules linked in: tcs3472(-) industrialio_triggered_buffer kfifo_buf industrialio ch341_buses binfmt_misc snd_hda_codec_realtek snd_hda_codec_generic ledtrig_audio snd_hda_codec_hdmi snd_hda_intel snd_intel_dspcfg snd_hda_codec snd_hwdep snd_hda_core snd_pcm wmi_bmof snd_seq snd_timer snd_seq_device input_leds rapl snd ccp k10temp soundcore wmi mac_hid sch_fq_codel parport_pc ppdev lp parport ip_tables x_tables autofs4 dm_crypt hid_generic usbhid hid radeon i2c_algo_bit drm_ttm_helper ttm drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops cec rc_core drm crct10dif_pclmul crc32_pclmul ghash_clmulni_intel aesni_intel r8169 crypto_simd cryptd ahci i2c_piix4 xhci_pci realtek libahci xhci_pci_renesas gpio_amdpt gpio_generic [last unloaded: tcs3472]
> [  157.008167] ---[ end trace c8eef7a3c5a40ff0 ]---
> 
> Signed-off-by: frank zago <frank@zago.net>

Hi Frank,

Already have a patch queued up that is identical to this one.
It just came in a tiny bit late wrt to merge window so will go
upstream in a week or two.

Thanks,

Jonathan



> ---
>  drivers/iio/industrialio-core.c | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index d92c58a94fe4..9e59f5da3d28 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -1926,9 +1926,6 @@ EXPORT_SYMBOL(__iio_device_register);
>   **/
>  void iio_device_unregister(struct iio_dev *indio_dev)
>  {
> -	struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev);
> -	struct iio_ioctl_handler *h, *t;
> -
>  	cdev_device_del(&indio_dev->chrdev, &indio_dev->dev);
>  
>  	mutex_lock(&indio_dev->info_exist_lock);
> @@ -1939,9 +1936,6 @@ void iio_device_unregister(struct iio_dev *indio_dev)
>  
>  	indio_dev->info = NULL;
>  
> -	list_for_each_entry_safe(h, t, &iio_dev_opaque->ioctl_handlers, entry)
> -		list_del(&h->entry);
> -
>  	iio_device_wakeup_eventset(indio_dev);
>  	iio_buffer_wakeup_poll(indio_dev);
>  


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] iio: light: tcs3472: do not free unallocated IRQ
  2021-04-27  2:20 ` [PATCH 2/2] iio: light: tcs3472: do not free unallocated IRQ Frank Zago
@ 2021-04-27 17:23   ` Jonathan Cameron
  2021-06-16 14:01     ` Jonathan Cameron
  0 siblings, 1 reply; 5+ messages in thread
From: Jonathan Cameron @ 2021-04-27 17:23 UTC (permalink / raw)
  To: Frank Zago
  Cc: Lars-Peter Clausen, Alexandru Ardelean, linux-iio, linux-kernel,
	Peter Meerwald

On Mon, 26 Apr 2021 21:20:17 -0500
Frank Zago <frank@zago.net> wrote:

> From: frank zago <frank@zago.net>
> 
> Allocating an IRQ is conditional to the IRQ existence, but freeing it
> was not. If no IRQ was allocate, the driver would still try to free
> IRQ 0. Add the missing checks.
> 
> This fixes the following trace when the driver is removed:
> 
> [  100.667788] Trying to free already-free IRQ 0
> [  100.667793] WARNING: CPU: 0 PID: 2315 at kernel/irq/manage.c:1826 free_irq+0x1fd/0x370
> [  100.667804] Modules linked in: tcs3472(-) industrialio_triggered_buffer kfifo_buf industrialio ch341_buses binfmt_misc snd_hda_codec_realtek snd_hda_codec_generic ledtrig_audio snd_hda_codec_hdmi snd_hda_intel snd_intel_dspcfg snd_hda_codec snd_hwdep snd_hda_core wmi_bmof snd_pcm snd_seq rapl input_leds snd_timer snd_seq_device snd k10temp ccp soundcore wmi mac_hid sch_fq_codel parport_pc ppdev lp parport ip_tables x_tables autofs4 dm_crypt hid_generic usbhid hid radeon i2c_algo_bit drm_ttm_helper ttm drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops cec rc_core drm crct10dif_pclmul crc32_pclmul ghash_clmulni_intel aesni_intel crypto_simd r8169 cryptd ahci i2c_piix4 xhci_pci realtek libahci xhci_pci_renesas gpio_amdpt gpio_generic
> [  100.667874] CPU: 0 PID: 2315 Comm: rmmod Not tainted 5.12.0+ #29
> [  100.667878] ...
> [  100.667881] RIP: 0010:free_irq+0x1fd/0x370
> [  100.667887] Code: e8 c8 d8 1b 00 48 83 c4 10 4c 89 f8 5b 41 5c 41 5d 41 5e 41 5f 5d c3 8b 75 d0 48 c7 c7 40 8b 36 93 4c 89 4d c8 e8 d1 2c a2 00 <0f> 0b 4c 8b 4d c8 4c 89 f7 4c 89 ce e8 72 c7 a8 00 49 8b 47 40 48
> [  100.667891] RSP: 0018:ffff9f44813b7d88 EFLAGS: 00010082
> [  100.667895] RAX: 0000000000000000 RBX: ffff8e50caf47800 RCX: ffff8e53cea185c8
> [  100.667897] RDX: 00000000ffffffd8 RSI: 0000000000000027 RDI: ffff8e53cea185c0
> [  100.667900] RBP: ffff9f44813b7dc0 R08: 0000000000000000 R09: ffff9f44813b7b50
> [  100.667902] R10: ffff9f44813b7b48 R11: ffffffff93b53848 R12: ffff8e50c0125080
> [  100.667903] R13: ffff8e50c0136f60 R14: ffff8e50c0136ea4 R15: ffff8e50c0136e00
> [  100.667906] FS:  00007fa28b899540(0000) GS:ffff8e53cea00000(0000) knlGS:0000000000000000
> [  100.667909] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [  100.667911] CR2: 0000561851777818 CR3: 000000010633a000 CR4: 00000000003506f0
> [  100.667914] Call Trace:
> [  100.667920]  tcs3472_remove+0x3a/0x90 [tcs3472]
> [  100.667927]  i2c_device_remove+0x2b/0xa0
> [  100.667934]  __device_release_driver+0x181/0x240
> [  100.667940]  driver_detach+0xd5/0x120
> [  100.667944]  bus_remove_driver+0x5c/0xe0
> [  100.667947]  driver_unregister+0x31/0x50
> [  100.667951]  i2c_del_driver+0x46/0x70
> [  100.667955]  tcs3472_driver_exit+0x10/0x5dd [tcs3472]
> [  100.667960]  __do_sys_delete_module.constprop.0+0x183/0x290
> [  100.667965]  ? exit_to_user_mode_prepare+0x37/0x1c0
> [  100.667971]  __x64_sys_delete_module+0x12/0x20
> [  100.667974]  do_syscall_64+0x40/0xb0
> [  100.667981]  entry_SYSCALL_64_after_hwframe+0x44/0xae
> [  100.667985] RIP: 0033:0x7fa28b9dbceb
> [  100.667989] Code: 73 01 c3 48 8b 0d 7d 91 0c 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa b8 b0 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 4d 91 0c 00 f7 d8 64 89 01 48
> [  100.667992] RSP: 002b:00007ffe02ea7068 EFLAGS: 00000206 ORIG_RAX: 00000000000000b0
> [  100.667995] RAX: ffffffffffffffda RBX: 000056185176d750 RCX: 00007fa28b9dbceb
> [  100.667997] RDX: 000000000000000a RSI: 0000000000000800 RDI: 000056185176d7b8
> [  100.667999] RBP: 00007ffe02ea70c8 R08: 0000000000000000 R09: 0000000000000000
> [  100.668001] R10: 00007fa28ba56ac0 R11: 0000000000000206 R12: 00007ffe02ea72a0
> [  100.668003] R13: 00007ffe02ea8807 R14: 000056185176d2a0 R15: 000056185176d750
> [  100.668007] ---[ end trace b21b0811931d933c ]---
> 
> Signed-off-by: frank zago <frank@zago.net>

Looks correct to me. +CC Peter in case he wants to take a look.

This is going to wait for a week or so anyway because next lot of fixes
will only go out towards the end of the merge window or just after rc1.

Thanks

Jonathan
> ---
>  drivers/iio/light/tcs3472.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/light/tcs3472.c b/drivers/iio/light/tcs3472.c
> index a0dc447aeb68..b41068492338 100644
> --- a/drivers/iio/light/tcs3472.c
> +++ b/drivers/iio/light/tcs3472.c
> @@ -531,7 +531,8 @@ static int tcs3472_probe(struct i2c_client *client,
>  	return 0;
>  
>  free_irq:
> -	free_irq(client->irq, indio_dev);
> +	if (client->irq)
> +		free_irq(client->irq, indio_dev);
>  buffer_cleanup:
>  	iio_triggered_buffer_cleanup(indio_dev);
>  	return ret;
> @@ -559,7 +560,8 @@ static int tcs3472_remove(struct i2c_client *client)
>  	struct iio_dev *indio_dev = i2c_get_clientdata(client);
>  
>  	iio_device_unregister(indio_dev);
> -	free_irq(client->irq, indio_dev);
> +	if (client->irq)
> +		free_irq(client->irq, indio_dev);
>  	iio_triggered_buffer_cleanup(indio_dev);
>  	tcs3472_powerdown(iio_priv(indio_dev));
>  


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] iio: light: tcs3472: do not free unallocated IRQ
  2021-04-27 17:23   ` Jonathan Cameron
@ 2021-06-16 14:01     ` Jonathan Cameron
  0 siblings, 0 replies; 5+ messages in thread
From: Jonathan Cameron @ 2021-06-16 14:01 UTC (permalink / raw)
  To: Frank Zago
  Cc: Lars-Peter Clausen, Alexandru Ardelean, linux-iio, linux-kernel,
	Peter Meerwald

On Tue, 27 Apr 2021 18:23:00 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Mon, 26 Apr 2021 21:20:17 -0500
> Frank Zago <frank@zago.net> wrote:
> 
> > From: frank zago <frank@zago.net>
> > 
> > Allocating an IRQ is conditional to the IRQ existence, but freeing it
> > was not. If no IRQ was allocate, the driver would still try to free
> > IRQ 0. Add the missing checks.
> > 
> > This fixes the following trace when the driver is removed:
> > 
> > [  100.667788] Trying to free already-free IRQ 0
> > [  100.667793] WARNING: CPU: 0 PID: 2315 at kernel/irq/manage.c:1826 free_irq+0x1fd/0x370
> > [  100.667804] Modules linked in: tcs3472(-) industrialio_triggered_buffer kfifo_buf industrialio ch341_buses binfmt_misc snd_hda_codec_realtek snd_hda_codec_generic ledtrig_audio snd_hda_codec_hdmi snd_hda_intel snd_intel_dspcfg snd_hda_codec snd_hwdep snd_hda_core wmi_bmof snd_pcm snd_seq rapl input_leds snd_timer snd_seq_device snd k10temp ccp soundcore wmi mac_hid sch_fq_codel parport_pc ppdev lp parport ip_tables x_tables autofs4 dm_crypt hid_generic usbhid hid radeon i2c_algo_bit drm_ttm_helper ttm drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops cec rc_core drm crct10dif_pclmul crc32_pclmul ghash_clmulni_intel aesni_intel crypto_simd r8169 cryptd ahci i2c_piix4 xhci_pci realtek libahci xhci_pci_renesas gpio_amdpt gpio_generic
> > [  100.667874] CPU: 0 PID: 2315 Comm: rmmod Not tainted 5.12.0+ #29
> > [  100.667878] ...
> > [  100.667881] RIP: 0010:free_irq+0x1fd/0x370
> > [  100.667887] Code: e8 c8 d8 1b 00 48 83 c4 10 4c 89 f8 5b 41 5c 41 5d 41 5e 41 5f 5d c3 8b 75 d0 48 c7 c7 40 8b 36 93 4c 89 4d c8 e8 d1 2c a2 00 <0f> 0b 4c 8b 4d c8 4c 89 f7 4c 89 ce e8 72 c7 a8 00 49 8b 47 40 48
> > [  100.667891] RSP: 0018:ffff9f44813b7d88 EFLAGS: 00010082
> > [  100.667895] RAX: 0000000000000000 RBX: ffff8e50caf47800 RCX: ffff8e53cea185c8
> > [  100.667897] RDX: 00000000ffffffd8 RSI: 0000000000000027 RDI: ffff8e53cea185c0
> > [  100.667900] RBP: ffff9f44813b7dc0 R08: 0000000000000000 R09: ffff9f44813b7b50
> > [  100.667902] R10: ffff9f44813b7b48 R11: ffffffff93b53848 R12: ffff8e50c0125080
> > [  100.667903] R13: ffff8e50c0136f60 R14: ffff8e50c0136ea4 R15: ffff8e50c0136e00
> > [  100.667906] FS:  00007fa28b899540(0000) GS:ffff8e53cea00000(0000) knlGS:0000000000000000
> > [  100.667909] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [  100.667911] CR2: 0000561851777818 CR3: 000000010633a000 CR4: 00000000003506f0
> > [  100.667914] Call Trace:
> > [  100.667920]  tcs3472_remove+0x3a/0x90 [tcs3472]
> > [  100.667927]  i2c_device_remove+0x2b/0xa0
> > [  100.667934]  __device_release_driver+0x181/0x240
> > [  100.667940]  driver_detach+0xd5/0x120
> > [  100.667944]  bus_remove_driver+0x5c/0xe0
> > [  100.667947]  driver_unregister+0x31/0x50
> > [  100.667951]  i2c_del_driver+0x46/0x70
> > [  100.667955]  tcs3472_driver_exit+0x10/0x5dd [tcs3472]
> > [  100.667960]  __do_sys_delete_module.constprop.0+0x183/0x290
> > [  100.667965]  ? exit_to_user_mode_prepare+0x37/0x1c0
> > [  100.667971]  __x64_sys_delete_module+0x12/0x20
> > [  100.667974]  do_syscall_64+0x40/0xb0
> > [  100.667981]  entry_SYSCALL_64_after_hwframe+0x44/0xae
> > [  100.667985] RIP: 0033:0x7fa28b9dbceb
> > [  100.667989] Code: 73 01 c3 48 8b 0d 7d 91 0c 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa b8 b0 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 4d 91 0c 00 f7 d8 64 89 01 48
> > [  100.667992] RSP: 002b:00007ffe02ea7068 EFLAGS: 00000206 ORIG_RAX: 00000000000000b0
> > [  100.667995] RAX: ffffffffffffffda RBX: 000056185176d750 RCX: 00007fa28b9dbceb
> > [  100.667997] RDX: 000000000000000a RSI: 0000000000000800 RDI: 000056185176d7b8
> > [  100.667999] RBP: 00007ffe02ea70c8 R08: 0000000000000000 R09: 0000000000000000
> > [  100.668001] R10: 00007fa28ba56ac0 R11: 0000000000000206 R12: 00007ffe02ea72a0
> > [  100.668003] R13: 00007ffe02ea8807 R14: 000056185176d2a0 R15: 000056185176d750
> > [  100.668007] ---[ end trace b21b0811931d933c ]---
> > 
> > Signed-off-by: frank zago <frank@zago.net>  
> 
> Looks correct to me. +CC Peter in case he wants to take a look.
> 
> This is going to wait for a week or so anyway because next lot of fixes
> will only go out towards the end of the merge window or just after rc1.
> 
> Thanks
> 
Frank,

Thank for the reminder. I had indeed lost track of this one.

Anyhow, now  applied to the togreg branch of iio.git as yet again we
are on the edge of a merge window, but this time we can just add this
to the last set of IIO patches to be lined up for that merge window.

Applied to the togreg branch of iio.git.

Thanks,

Jonathan

> Jonathan
> > ---
> >  drivers/iio/light/tcs3472.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/iio/light/tcs3472.c b/drivers/iio/light/tcs3472.c
> > index a0dc447aeb68..b41068492338 100644
> > --- a/drivers/iio/light/tcs3472.c
> > +++ b/drivers/iio/light/tcs3472.c
> > @@ -531,7 +531,8 @@ static int tcs3472_probe(struct i2c_client *client,
> >  	return 0;
> >  
> >  free_irq:
> > -	free_irq(client->irq, indio_dev);
> > +	if (client->irq)
> > +		free_irq(client->irq, indio_dev);
> >  buffer_cleanup:
> >  	iio_triggered_buffer_cleanup(indio_dev);
> >  	return ret;
> > @@ -559,7 +560,8 @@ static int tcs3472_remove(struct i2c_client *client)
> >  	struct iio_dev *indio_dev = i2c_get_clientdata(client);
> >  
> >  	iio_device_unregister(indio_dev);
> > -	free_irq(client->irq, indio_dev);
> > +	if (client->irq)
> > +		free_irq(client->irq, indio_dev);
> >  	iio_triggered_buffer_cleanup(indio_dev);
> >  	tcs3472_powerdown(iio_priv(indio_dev));
> >    
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-06-16 13:59 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-27  2:20 [PATCH 1/2] iio: core: fix list breakage in iio_device_unregister() Frank Zago
2021-04-27  2:20 ` [PATCH 2/2] iio: light: tcs3472: do not free unallocated IRQ Frank Zago
2021-04-27 17:23   ` Jonathan Cameron
2021-06-16 14:01     ` Jonathan Cameron
2021-04-27  8:23 ` [PATCH 1/2] iio: core: fix list breakage in iio_device_unregister() Jonathan Cameron

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.