* ir_kbd_i2c oops
@ 2019-11-18 16:21 Nick French
2019-11-21 6:44 ` Nick French
0 siblings, 1 reply; 5+ messages in thread
From: Nick French @ 2019-11-18 16:21 UTC (permalink / raw)
To: linux-media
Hello all,
I'm about to start trying to track down an intermittent oops in
ir_kbd_i2c that I've been having for a long time over various recent
kernels. It must be some kind of timing/race condition at startup,
because I only get it about 5% of boots.
Here's the relevant snippet from the log, if anyone has any smart
ideas let me know. I should have better debugging data within the next
week or so.
...
Registered IR keymap rc-hauppauge
rc rc0: Hauppauge WinTV PVR-350 as
/devices/pci0000:00/0000:00:1e.0/0000:04:00.0/i2c-0/0-0018/rc/rc0
input: Hauppauge WinTV PVR-350 as
/devices/pci0000:00/0000:00:1e.0/0000:04:00.0/i2c-0/0-0018/rc/rc0/input9
BUG: kernel NULL pointer dereference, address: 0000000000000038
#PF: supervisor read access in kernel mode
#PF: error_code(0x0000) - not-present page
PGD 0 P4D 0
Oops: 0000 [#1] SMP PTI
CPU: 1 PID: 17 Comm: kworker/1:0 Not tainted 5.3.11-300.fc31.x86_64 #1
Hardware name: /DG43NB, BIOS NBG4310H.86A.0096.2009.0903.1845 09/03/2009
Workqueue: events ir_work [ir_kbd_i2c]
RIP: 0010:ir_lirc_scancode_event+0x3d/0xb0
Code: a6 b4 07 00 00 49 81 c6 b8 07 00 00 55 53 e8 ba a7 9d ff 4c 89
e7 49 89 45 00 e8 5e 7a 25 00 49 8b 1e 48 89 c5 4c 39 f3 74 58 <8b> 43
38 8b 53 40 89 c1 2b 4b 3c 39 ca 72 41 21 d0 49 8b 7d 00 49
RSP: 0018:ffffaae2000b3d88 EFLAGS: 00010017
RAX: 0000000000000002 RBX: 0000000000000000 RCX: 0000000000000019
RDX: 0000000000000001 RSI: 006e801b1f26ce6a RDI: ffff9e39797c37b4
RBP: 0000000000000002 R08: 0000000000000001 R09: 0000000000000001
R10: 0000000000000001 R11: 0000000000000001 R12: ffff9e39797c37b4
R13: ffffaae2000b3db8 R14: ffff9e39797c37b8 R15: ffff9e39797c33d8
FS: 0000000000000000(0000) GS:ffff9e397b680000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000038 CR3: 0000000035844000 CR4: 00000000000006e0
Call Trace:
ir_do_keydown+0x8e/0x2b0
rc_keydown+0x52/0xc0
ir_work+0xb8/0x130 [ir_kbd_i2c]
process_one_work+0x19d/0x340
worker_thread+0x50/0x3b0
kthread+0xfb/0x130
? process_one_work+0x340/0x340
? kthread_park+0x80/0x80
ret_from_fork+0x35/0x40
Modules linked in: rc_hauppauge tuner msp3400 saa7127 saa7115 ivtv(+)
tveeprom cx2341x v4l2_common videodev mc i2c_algo_bit ir_kbd_i2c
ip_tables firewire_ohci e1000e serio_raw firewire_core ata_generic
crc_itu_t pata_acpi pata_jmicron fuse
CR2: 0000000000000038
---[ end trace c67c2697a99fa74b ]---
RIP: 0010:ir_lirc_scancode_event+0x3d/0xb0
Code: a6 b4 07 00 00 49 81 c6 b8 07 00 00 55 53 e8 ba a7 9d ff 4c 89
e7 49 89 45 00 e8 5e 7a 25 00 49 8b 1e 48 89 c5 4c 39 f3 74 58 <8b> 43
38 8b 53 40 89 c1 2b 4b 3c 39 ca 72 41 21 d0 49 8b 7d 00 49
RSP: 0018:ffffaae2000b3d88 EFLAGS: 00010017
RAX: 0000000000000002 RBX: 0000000000000000 RCX: 0000000000000019
RDX: 0000000000000001 RSI: 006e801b1f26ce6a RDI: ffff9e39797c37b4
RBP: 0000000000000002 R08: 0000000000000001 R09: 0000000000000001
R10: 0000000000000001 R11: 0000000000000001 R12: ffff9e39797c37b4
R13: ffffaae2000b3db8 R14: ffff9e39797c37b8 R15: ffff9e39797c33d8
FS: 0000000000000000(0000) GS:ffff9e397b680000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000038 CR3: 0000000035844000 CR4: 00000000000006e0
rc rc0: lirc_dev: driver ir_kbd_i2c registered at minor = 0, scancode
receiver, no transmitter
tuner-simple 0-0061: creating new instance
tuner-simple 0-0061: type set to 2 (Philips NTSC (FI1236,FM1236 and
compatibles))
ivtv0: Registered device video0 for encoder MPG (4096 kB)
ivtv0: Registered device video32 for encoder YUV (2048 kB)
ivtv0: Registered device vbi0 for encoder VBI (1024 kB)
ivtv0: Registered device video24 for encoder PCM (320 kB)
ivtv0: Registered device radio0 for encoder radio
ivtv0: Registered device video16 for decoder MPG (1024 kB)
ivtv0: Registered device vbi8 for decoder VBI (64 kB)
ivtv0: Registered device vbi16 for decoder VOUT
ivtv0: Registered device video48 for decoder YUV (1024 kB)
...
- Nick
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: ir_kbd_i2c oops
2019-11-18 16:21 ir_kbd_i2c oops Nick French
@ 2019-11-21 6:44 ` Nick French
2019-11-21 10:24 ` Sean Young
0 siblings, 1 reply; 5+ messages in thread
From: Nick French @ 2019-11-21 6:44 UTC (permalink / raw)
To: linux-media; +Cc: kraxel, mkochano, lirc, ulrich.mueller42, jarod
On Mon, Nov 18, 2019 at 10:21 AM Nick French <nickfrench@gmail.com> wrote:
>
> Hello all,
>
> I'm about to start trying to track down an intermittent oops in
> ir_kbd_i2c that I've been having for a long time over various recent
> kernels. It must be some kind of timing/race condition at startup,
> because I only get it about 5% of boots.
>
> Here's the relevant snippet from the log, if anyone has any smart
> ideas let me know. I should have better debugging data within the next
> week or so.
>
> ...
> Registered IR keymap rc-hauppauge
> rc rc0: Hauppauge WinTV PVR-350 as
> /devices/pci0000:00/0000:00:1e.0/0000:04:00.0/i2c-0/0-0018/rc/rc0
> input: Hauppauge WinTV PVR-350 as
> /devices/pci0000:00/0000:00:1e.0/0000:04:00.0/i2c-0/0-0018/rc/rc0/input9
> BUG: kernel NULL pointer dereference, address: 0000000000000038
> #PF: supervisor read access in kernel mode
> #PF: error_code(0x0000) - not-present page
> PGD 0 P4D 0
> Oops: 0000 [#1] SMP PTI
> CPU: 1 PID: 17 Comm: kworker/1:0 Not tainted 5.3.11-300.fc31.x86_64 #1
> Hardware name: /DG43NB, BIOS NBG4310H.86A.0096.2009.0903.1845 09/03/2009
> Workqueue: events ir_work [ir_kbd_i2c]
> RIP: 0010:ir_lirc_scancode_event+0x3d/0xb0
> Code: a6 b4 07 00 00 49 81 c6 b8 07 00 00 55 53 e8 ba a7 9d ff 4c 89
> e7 49 89 45 00 e8 5e 7a 25 00 49 8b 1e 48 89 c5 4c 39 f3 74 58 <8b> 43
> 38 8b 53 40 89 c1 2b 4b 3c 39 ca 72 41 21 d0 49 8b 7d 00 49
> RSP: 0018:ffffaae2000b3d88 EFLAGS: 00010017
> RAX: 0000000000000002 RBX: 0000000000000000 RCX: 0000000000000019
> RDX: 0000000000000001 RSI: 006e801b1f26ce6a RDI: ffff9e39797c37b4
> RBP: 0000000000000002 R08: 0000000000000001 R09: 0000000000000001
> R10: 0000000000000001 R11: 0000000000000001 R12: ffff9e39797c37b4
> R13: ffffaae2000b3db8 R14: ffff9e39797c37b8 R15: ffff9e39797c33d8
> FS: 0000000000000000(0000) GS:ffff9e397b680000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000000000038 CR3: 0000000035844000 CR4: 00000000000006e0
> Call Trace:
> ir_do_keydown+0x8e/0x2b0
> rc_keydown+0x52/0xc0
> ir_work+0xb8/0x130 [ir_kbd_i2c]
> process_one_work+0x19d/0x340
> worker_thread+0x50/0x3b0
> kthread+0xfb/0x130
> ? process_one_work+0x340/0x340
> ? kthread_park+0x80/0x80
> ret_from_fork+0x35/0x40
> Modules linked in: rc_hauppauge tuner msp3400 saa7127 saa7115 ivtv(+)
> tveeprom cx2341x v4l2_common videodev mc i2c_algo_bit ir_kbd_i2c
> ip_tables firewire_ohci e1000e serio_raw firewire_core ata_generic
> crc_itu_t pata_acpi pata_jmicron fuse
> CR2: 0000000000000038
> ---[ end trace c67c2697a99fa74b ]---
> RIP: 0010:ir_lirc_scancode_event+0x3d/0xb0
> Code: a6 b4 07 00 00 49 81 c6 b8 07 00 00 55 53 e8 ba a7 9d ff 4c 89
> e7 49 89 45 00 e8 5e 7a 25 00 49 8b 1e 48 89 c5 4c 39 f3 74 58 <8b> 43
> 38 8b 53 40 89 c1 2b 4b 3c 39 ca 72 41 21 d0 49 8b 7d 00 49
> RSP: 0018:ffffaae2000b3d88 EFLAGS: 00010017
> RAX: 0000000000000002 RBX: 0000000000000000 RCX: 0000000000000019
> RDX: 0000000000000001 RSI: 006e801b1f26ce6a RDI: ffff9e39797c37b4
> RBP: 0000000000000002 R08: 0000000000000001 R09: 0000000000000001
> R10: 0000000000000001 R11: 0000000000000001 R12: ffff9e39797c37b4
> R13: ffffaae2000b3db8 R14: ffff9e39797c37b8 R15: ffff9e39797c33d8
> FS: 0000000000000000(0000) GS:ffff9e397b680000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000000000038 CR3: 0000000035844000 CR4: 00000000000006e0
> rc rc0: lirc_dev: driver ir_kbd_i2c registered at minor = 0, scancode
> receiver, no transmitter
> tuner-simple 0-0061: creating new instance
> tuner-simple 0-0061: type set to 2 (Philips NTSC (FI1236,FM1236 and
> compatibles))
> ivtv0: Registered device video0 for encoder MPG (4096 kB)
> ivtv0: Registered device video32 for encoder YUV (2048 kB)
> ivtv0: Registered device vbi0 for encoder VBI (1024 kB)
> ivtv0: Registered device video24 for encoder PCM (320 kB)
> ivtv0: Registered device radio0 for encoder radio
> ivtv0: Registered device video16 for decoder MPG (1024 kB)
> ivtv0: Registered device vbi8 for decoder VBI (64 kB)
> ivtv0: Registered device vbi16 for decoder VOUT
> ivtv0: Registered device video48 for decoder YUV (1024 kB)
> ...
>
> - Nick
I haven't been able to recreate with a debug build yet, but I think I
see the problem:
ir_kbd_i2c calls rc_register_device(), where the ordering of calls is this:
dev->registered = true;
rc_setup_rx_device(dev); <--calls rc_open()
ir_lirc_register(dev); <--initializes lirc_fh/lirc_fh_lock
however, ir_kbd_i2c's rc_open() callback schedules work whose
call-stack looks like:
ir_work()
ir_key_poll()
rc_keydown()
ir_do_keydown()
ir_lirc_scancode_event() <-- uses lirc_fh/lirc_hf_lock
So basically if there is a keydown detection *during* ir_kbd_i2c
initialization, its going to oops accessing the uninitialized
lirc_fh_lock, I think?
Not sure what to do with that info though, even if correct.
An 'is registered' check does no good, because the device is marked
registered before rc_setup_rx_device() happens. In fact rc_open()
already checks this.
Hoping someone smarter than me can weigh in here...
Thanks,
Nick
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: ir_kbd_i2c oops
2019-11-21 6:44 ` Nick French
@ 2019-11-21 10:24 ` Sean Young
2019-11-21 14:19 ` Nick French
0 siblings, 1 reply; 5+ messages in thread
From: Sean Young @ 2019-11-21 10:24 UTC (permalink / raw)
To: Nick French; +Cc: linux-media, kraxel, mkochano, lirc, ulrich.mueller42, jarod
Hi Nick,
On Thu, Nov 21, 2019 at 12:44:42AM -0600, Nick French wrote:
> On Mon, Nov 18, 2019 at 10:21 AM Nick French <nickfrench@gmail.com> wrote:
> > I'm about to start trying to track down an intermittent oops in
> > ir_kbd_i2c that I've been having for a long time over various recent
> > kernels. It must be some kind of timing/race condition at startup,
> > because I only get it about 5% of boots.
> >
> > Here's the relevant snippet from the log, if anyone has any smart
> > ideas let me know. I should have better debugging data within the next
> > week or so.
> >
> > ...
> > Registered IR keymap rc-hauppauge
> > rc rc0: Hauppauge WinTV PVR-350 as
> > /devices/pci0000:00/0000:00:1e.0/0000:04:00.0/i2c-0/0-0018/rc/rc0
> > input: Hauppauge WinTV PVR-350 as
> > /devices/pci0000:00/0000:00:1e.0/0000:04:00.0/i2c-0/0-0018/rc/rc0/input9
> > BUG: kernel NULL pointer dereference, address: 0000000000000038
> > #PF: supervisor read access in kernel mode
> > #PF: error_code(0x0000) - not-present page
> > PGD 0 P4D 0
> > Oops: 0000 [#1] SMP PTI
> > CPU: 1 PID: 17 Comm: kworker/1:0 Not tainted 5.3.11-300.fc31.x86_64 #1
> > Hardware name: /DG43NB, BIOS NBG4310H.86A.0096.2009.0903.1845 09/03/2009
> > Workqueue: events ir_work [ir_kbd_i2c]
> > RIP: 0010:ir_lirc_scancode_event+0x3d/0xb0
> > Code: a6 b4 07 00 00 49 81 c6 b8 07 00 00 55 53 e8 ba a7 9d ff 4c 89
> > e7 49 89 45 00 e8 5e 7a 25 00 49 8b 1e 48 89 c5 4c 39 f3 74 58 <8b> 43
> > 38 8b 53 40 89 c1 2b 4b 3c 39 ca 72 41 21 d0 49 8b 7d 00 49
> > RSP: 0018:ffffaae2000b3d88 EFLAGS: 00010017
> > RAX: 0000000000000002 RBX: 0000000000000000 RCX: 0000000000000019
> > RDX: 0000000000000001 RSI: 006e801b1f26ce6a RDI: ffff9e39797c37b4
> > RBP: 0000000000000002 R08: 0000000000000001 R09: 0000000000000001
> > R10: 0000000000000001 R11: 0000000000000001 R12: ffff9e39797c37b4
> > R13: ffffaae2000b3db8 R14: ffff9e39797c37b8 R15: ffff9e39797c33d8
> > FS: 0000000000000000(0000) GS:ffff9e397b680000(0000) knlGS:0000000000000000
> > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > CR2: 0000000000000038 CR3: 0000000035844000 CR4: 00000000000006e0
> > Call Trace:
> > ir_do_keydown+0x8e/0x2b0
> > rc_keydown+0x52/0xc0
> > ir_work+0xb8/0x130 [ir_kbd_i2c]
> > process_one_work+0x19d/0x340
> > worker_thread+0x50/0x3b0
> > kthread+0xfb/0x130
> > ? process_one_work+0x340/0x340
> > ? kthread_park+0x80/0x80
> > ret_from_fork+0x35/0x40
> > Modules linked in: rc_hauppauge tuner msp3400 saa7127 saa7115 ivtv(+)
> > tveeprom cx2341x v4l2_common videodev mc i2c_algo_bit ir_kbd_i2c
> > ip_tables firewire_ohci e1000e serio_raw firewire_core ata_generic
> > crc_itu_t pata_acpi pata_jmicron fuse
> > CR2: 0000000000000038
> > ---[ end trace c67c2697a99fa74b ]---
> > RIP: 0010:ir_lirc_scancode_event+0x3d/0xb0
> > Code: a6 b4 07 00 00 49 81 c6 b8 07 00 00 55 53 e8 ba a7 9d ff 4c 89
> > e7 49 89 45 00 e8 5e 7a 25 00 49 8b 1e 48 89 c5 4c 39 f3 74 58 <8b> 43
> > 38 8b 53 40 89 c1 2b 4b 3c 39 ca 72 41 21 d0 49 8b 7d 00 49
> > RSP: 0018:ffffaae2000b3d88 EFLAGS: 00010017
> > RAX: 0000000000000002 RBX: 0000000000000000 RCX: 0000000000000019
> > RDX: 0000000000000001 RSI: 006e801b1f26ce6a RDI: ffff9e39797c37b4
> > RBP: 0000000000000002 R08: 0000000000000001 R09: 0000000000000001
> > R10: 0000000000000001 R11: 0000000000000001 R12: ffff9e39797c37b4
> > R13: ffffaae2000b3db8 R14: ffff9e39797c37b8 R15: ffff9e39797c33d8
> > FS: 0000000000000000(0000) GS:ffff9e397b680000(0000) knlGS:0000000000000000
> > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > CR2: 0000000000000038 CR3: 0000000035844000 CR4: 00000000000006e0
> > rc rc0: lirc_dev: driver ir_kbd_i2c registered at minor = 0, scancode
> > receiver, no transmitter
> > tuner-simple 0-0061: creating new instance
> > tuner-simple 0-0061: type set to 2 (Philips NTSC (FI1236,FM1236 and
> > compatibles))
> > ivtv0: Registered device video0 for encoder MPG (4096 kB)
> > ivtv0: Registered device video32 for encoder YUV (2048 kB)
> > ivtv0: Registered device vbi0 for encoder VBI (1024 kB)
> > ivtv0: Registered device video24 for encoder PCM (320 kB)
> > ivtv0: Registered device radio0 for encoder radio
> > ivtv0: Registered device video16 for decoder MPG (1024 kB)
> > ivtv0: Registered device vbi8 for decoder VBI (64 kB)
> > ivtv0: Registered device vbi16 for decoder VOUT
> > ivtv0: Registered device video48 for decoder YUV (1024 kB)
> > ...
> >
> > - Nick
>
> I haven't been able to recreate with a debug build yet, but I think I
> see the problem:
>
> ir_kbd_i2c calls rc_register_device(), where the ordering of calls is this:
> dev->registered = true;
> rc_setup_rx_device(dev); <--calls rc_open()
> ir_lirc_register(dev); <--initializes lirc_fh/lirc_fh_lock
>
> however, ir_kbd_i2c's rc_open() callback schedules work whose
> call-stack looks like:
> ir_work()
> ir_key_poll()
> rc_keydown()
> ir_do_keydown()
> ir_lirc_scancode_event() <-- uses lirc_fh/lirc_hf_lock
>
> So basically if there is a keydown detection *during* ir_kbd_i2c
> initialization, its going to oops accessing the uninitialized
> lirc_fh_lock, I think?
>
> Not sure what to do with that info though, even if correct.
> An 'is registered' check does no good, because the device is marked
> registered before rc_setup_rx_device() happens. In fact rc_open()
> already checks this.
>
> Hoping someone smarter than me can weigh in here...
I think you have figured it out exactly. That's a highly unlikely race
condition; in rc_register_device(), in between rc_setup_rx_device()
and ir_lirc_register() user space must open the input device, and the
ir_work must be scheduled and execute as well.
The comment on ir_lirc_register() alludes to the same problem, which
I considered for raw IR devices, but obviously not for scancode devices.
It would be very useful to have this patch tested, I have never seen
this race condition.
Thanks!
Sean
From d14f6695982b86b383063e74fc6a4febf29248f3 Mon Sep 17 00:00:00 2001
From: Sean Young <sean@mess.org>
Date: Thu, 21 Nov 2019 10:10:47 +0000
Subject: [PATCH] media: rc: ensure lirc is initialized before registering
input device
Once rc_open is called on the input device, lirc events can be delivered.
Ensure lirc is ready to do so else we might get this:
Registered IR keymap rc-hauppauge
rc rc0: Hauppauge WinTV PVR-350 as
/devices/pci0000:00/0000:00:1e.0/0000:04:00.0/i2c-0/0-0018/rc/rc0
input: Hauppauge WinTV PVR-350 as
/devices/pci0000:00/0000:00:1e.0/0000:04:00.0/i2c-0/0-0018/rc/rc0/input9
BUG: kernel NULL pointer dereference, address: 0000000000000038
PGD 0 P4D 0
Oops: 0000 [#1] SMP PTI
CPU: 1 PID: 17 Comm: kworker/1:0 Not tainted 5.3.11-300.fc31.x86_64 #1
Hardware name: /DG43NB, BIOS NBG4310H.86A.0096.2009.0903.1845 09/03/2009
Workqueue: events ir_work [ir_kbd_i2c]
RIP: 0010:ir_lirc_scancode_event+0x3d/0xb0
Code: a6 b4 07 00 00 49 81 c6 b8 07 00 00 55 53 e8 ba a7 9d ff 4c 89
e7 49 89 45 00 e8 5e 7a 25 00 49 8b 1e 48 89 c5 4c 39 f3 74 58 <8b> 43
38 8b 53 40 89 c1 2b 4b 3c 39 ca 72 41 21 d0 49 8b 7d 00 49
RSP: 0018:ffffaae2000b3d88 EFLAGS: 00010017
RAX: 0000000000000002 RBX: 0000000000000000 RCX: 0000000000000019
RDX: 0000000000000001 RSI: 006e801b1f26ce6a RDI: ffff9e39797c37b4
RBP: 0000000000000002 R08: 0000000000000001 R09: 0000000000000001
R10: 0000000000000001 R11: 0000000000000001 R12: ffff9e39797c37b4
R13: ffffaae2000b3db8 R14: ffff9e39797c37b8 R15: ffff9e39797c33d8
FS: 0000000000000000(0000) GS:ffff9e397b680000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000038 CR3: 0000000035844000 CR4: 00000000000006e0
Call Trace:
ir_do_keydown+0x8e/0x2b0
rc_keydown+0x52/0xc0
ir_work+0xb8/0x130 [ir_kbd_i2c]
process_one_work+0x19d/0x340
worker_thread+0x50/0x3b0
kthread+0xfb/0x130
? process_one_work+0x340/0x340
? kthread_park+0x80/0x80
ret_from_fork+0x35/0x40
Modules linked in: rc_hauppauge tuner msp3400 saa7127 saa7115 ivtv(+)
tveeprom cx2341x v4l2_common videodev mc i2c_algo_bit ir_kbd_i2c
ip_tables firewire_ohci e1000e serio_raw firewire_core ata_generic
crc_itu_t pata_acpi pata_jmicron fuse
CR2: 0000000000000038
---[ end trace c67c2697a99fa74b ]---
RIP: 0010:ir_lirc_scancode_event+0x3d/0xb0
Code: a6 b4 07 00 00 49 81 c6 b8 07 00 00 55 53 e8 ba a7 9d ff 4c 89
e7 49 89 45 00 e8 5e 7a 25 00 49 8b 1e 48 89 c5 4c 39 f3 74 58 <8b> 43
38 8b 53 40 89 c1 2b 4b 3c 39 ca 72 41 21 d0 49 8b 7d 00 49
RSP: 0018:ffffaae2000b3d88 EFLAGS: 00010017
RAX: 0000000000000002 RBX: 0000000000000000 RCX: 0000000000000019
RDX: 0000000000000001 RSI: 006e801b1f26ce6a RDI: ffff9e39797c37b4
RBP: 0000000000000002 R08: 0000000000000001 R09: 0000000000000001
R10: 0000000000000001 R11: 0000000000000001 R12: ffff9e39797c37b4
R13: ffffaae2000b3db8 R14: ffff9e39797c37b8 R15: ffff9e39797c33d8
FS: 0000000000000000(0000) GS:ffff9e397b680000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000038 CR3: 0000000035844000 CR4: 00000000000006e0
rc rc0: lirc_dev: driver ir_kbd_i2c registered at minor = 0, scancode
receiver, no transmitter
tuner-simple 0-0061: creating new instance
tuner-simple 0-0061: type set to 2 (Philips NTSC (FI1236,FM1236 and
compatibles))
ivtv0: Registered device video0 for encoder MPG (4096 kB)
ivtv0: Registered device video32 for encoder YUV (2048 kB)
ivtv0: Registered device vbi0 for encoder VBI (1024 kB)
ivtv0: Registered device video24 for encoder PCM (320 kB)
ivtv0: Registered device radio0 for encoder radio
ivtv0: Registered device video16 for decoder MPG (1024 kB)
ivtv0: Registered device vbi8 for decoder VBI (64 kB)
ivtv0: Registered device vbi16 for decoder VOUT
ivtv0: Registered device video48 for decoder YUV (1024 kB)
Cc: stable@vger.kernel.org
Reported-by: Nick French <nickfrench@gmail.com>
Signed-off-by: Sean Young <sean@mess.org>
---
drivers/media/rc/rc-main.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index 7741151606ef..f29def4f35d8 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -1891,23 +1891,23 @@ int rc_register_device(struct rc_dev *dev)
dev->registered = true;
- if (dev->driver_type != RC_DRIVER_IR_RAW_TX) {
- rc = rc_setup_rx_device(dev);
- if (rc)
- goto out_dev;
- }
-
/* Ensure that the lirc kfifo is setup before we start the thread */
if (dev->allowed_protocols != RC_PROTO_BIT_CEC) {
rc = ir_lirc_register(dev);
if (rc < 0)
- goto out_rx;
+ goto out_dev;
+ }
+
+ if (dev->driver_type != RC_DRIVER_IR_RAW_TX) {
+ rc = rc_setup_rx_device(dev);
+ if (rc)
+ goto out_lirc;
}
if (dev->driver_type == RC_DRIVER_IR_RAW) {
rc = ir_raw_event_register(dev);
if (rc < 0)
- goto out_lirc;
+ goto out_rx;
}
dev_dbg(&dev->dev, "Registered rc%u (driver: %s)\n", dev->minor,
@@ -1915,11 +1915,11 @@ int rc_register_device(struct rc_dev *dev)
return 0;
+out_rx:
+ rc_free_rx_device(dev);
out_lirc:
if (dev->allowed_protocols != RC_PROTO_BIT_CEC)
ir_lirc_unregister(dev);
-out_rx:
- rc_free_rx_device(dev);
out_dev:
device_del(&dev->dev);
out_rx_free:
--
2.23.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: ir_kbd_i2c oops
2019-11-21 10:24 ` Sean Young
@ 2019-11-21 14:19 ` Nick French
2019-12-10 19:26 ` Nick French
0 siblings, 1 reply; 5+ messages in thread
From: Nick French @ 2019-11-21 14:19 UTC (permalink / raw)
To: Sean Young; +Cc: linux-media, kraxel, mkochano, lirc, ulrich.mueller42, jarod
On Thu, Nov 21, 2019 at 4:24 AM Sean Young <sean@mess.org> wrote:
>
> Hi Nick,
>
> On Thu, Nov 21, 2019 at 12:44:42AM -0600, Nick French wrote:
> > On Mon, Nov 18, 2019 at 10:21 AM Nick French <nickfrench@gmail.com> wrote:
> > > I'm about to start trying to track down an intermittent oops in
> > > ir_kbd_i2c that I've been having for a long time over various recent
> > > kernels. It must be some kind of timing/race condition at startup,
> > > because I only get it about 5% of boots.
> > >
> > > Here's the relevant snippet from the log, if anyone has any smart
> > > ideas let me know. I should have better debugging data within the next
> > > week or so.
> > >
> > > ...
> > > Registered IR keymap rc-hauppauge
> > > rc rc0: Hauppauge WinTV PVR-350 as
> > > /devices/pci0000:00/0000:00:1e.0/0000:04:00.0/i2c-0/0-0018/rc/rc0
> > > input: Hauppauge WinTV PVR-350 as
> > > /devices/pci0000:00/0000:00:1e.0/0000:04:00.0/i2c-0/0-0018/rc/rc0/input9
> > > BUG: kernel NULL pointer dereference, address: 0000000000000038
> > > #PF: supervisor read access in kernel mode
> > > #PF: error_code(0x0000) - not-present page
> > > PGD 0 P4D 0
> > > Oops: 0000 [#1] SMP PTI
> > > CPU: 1 PID: 17 Comm: kworker/1:0 Not tainted 5.3.11-300.fc31.x86_64 #1
> > > Hardware name: /DG43NB, BIOS NBG4310H.86A.0096.2009.0903.1845 09/03/2009
> > > Workqueue: events ir_work [ir_kbd_i2c]
> > > RIP: 0010:ir_lirc_scancode_event+0x3d/0xb0
> > > Code: a6 b4 07 00 00 49 81 c6 b8 07 00 00 55 53 e8 ba a7 9d ff 4c 89
> > > e7 49 89 45 00 e8 5e 7a 25 00 49 8b 1e 48 89 c5 4c 39 f3 74 58 <8b> 43
> > > 38 8b 53 40 89 c1 2b 4b 3c 39 ca 72 41 21 d0 49 8b 7d 00 49
> > > RSP: 0018:ffffaae2000b3d88 EFLAGS: 00010017
> > > RAX: 0000000000000002 RBX: 0000000000000000 RCX: 0000000000000019
> > > RDX: 0000000000000001 RSI: 006e801b1f26ce6a RDI: ffff9e39797c37b4
> > > RBP: 0000000000000002 R08: 0000000000000001 R09: 0000000000000001
> > > R10: 0000000000000001 R11: 0000000000000001 R12: ffff9e39797c37b4
> > > R13: ffffaae2000b3db8 R14: ffff9e39797c37b8 R15: ffff9e39797c33d8
> > > FS: 0000000000000000(0000) GS:ffff9e397b680000(0000) knlGS:0000000000000000
> > > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > CR2: 0000000000000038 CR3: 0000000035844000 CR4: 00000000000006e0
> > > Call Trace:
> > > ir_do_keydown+0x8e/0x2b0
> > > rc_keydown+0x52/0xc0
> > > ir_work+0xb8/0x130 [ir_kbd_i2c]
> > > process_one_work+0x19d/0x340
> > > worker_thread+0x50/0x3b0
> > > kthread+0xfb/0x130
> > > ? process_one_work+0x340/0x340
> > > ? kthread_park+0x80/0x80
> > > ret_from_fork+0x35/0x40
> > > Modules linked in: rc_hauppauge tuner msp3400 saa7127 saa7115 ivtv(+)
> > > tveeprom cx2341x v4l2_common videodev mc i2c_algo_bit ir_kbd_i2c
> > > ip_tables firewire_ohci e1000e serio_raw firewire_core ata_generic
> > > crc_itu_t pata_acpi pata_jmicron fuse
> > > CR2: 0000000000000038
> > > ---[ end trace c67c2697a99fa74b ]---
> > > RIP: 0010:ir_lirc_scancode_event+0x3d/0xb0
> > > Code: a6 b4 07 00 00 49 81 c6 b8 07 00 00 55 53 e8 ba a7 9d ff 4c 89
> > > e7 49 89 45 00 e8 5e 7a 25 00 49 8b 1e 48 89 c5 4c 39 f3 74 58 <8b> 43
> > > 38 8b 53 40 89 c1 2b 4b 3c 39 ca 72 41 21 d0 49 8b 7d 00 49
> > > RSP: 0018:ffffaae2000b3d88 EFLAGS: 00010017
> > > RAX: 0000000000000002 RBX: 0000000000000000 RCX: 0000000000000019
> > > RDX: 0000000000000001 RSI: 006e801b1f26ce6a RDI: ffff9e39797c37b4
> > > RBP: 0000000000000002 R08: 0000000000000001 R09: 0000000000000001
> > > R10: 0000000000000001 R11: 0000000000000001 R12: ffff9e39797c37b4
> > > R13: ffffaae2000b3db8 R14: ffff9e39797c37b8 R15: ffff9e39797c33d8
> > > FS: 0000000000000000(0000) GS:ffff9e397b680000(0000) knlGS:0000000000000000
> > > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > CR2: 0000000000000038 CR3: 0000000035844000 CR4: 00000000000006e0
> > > rc rc0: lirc_dev: driver ir_kbd_i2c registered at minor = 0, scancode
> > > receiver, no transmitter
> > > tuner-simple 0-0061: creating new instance
> > > tuner-simple 0-0061: type set to 2 (Philips NTSC (FI1236,FM1236 and
> > > compatibles))
> > > ivtv0: Registered device video0 for encoder MPG (4096 kB)
> > > ivtv0: Registered device video32 for encoder YUV (2048 kB)
> > > ivtv0: Registered device vbi0 for encoder VBI (1024 kB)
> > > ivtv0: Registered device video24 for encoder PCM (320 kB)
> > > ivtv0: Registered device radio0 for encoder radio
> > > ivtv0: Registered device video16 for decoder MPG (1024 kB)
> > > ivtv0: Registered device vbi8 for decoder VBI (64 kB)
> > > ivtv0: Registered device vbi16 for decoder VOUT
> > > ivtv0: Registered device video48 for decoder YUV (1024 kB)
> > > ...
> > >
> > > - Nick
> >
> > I haven't been able to recreate with a debug build yet, but I think I
> > see the problem:
> >
> > ir_kbd_i2c calls rc_register_device(), where the ordering of calls is this:
> > dev->registered = true;
> > rc_setup_rx_device(dev); <--calls rc_open()
> > ir_lirc_register(dev); <--initializes lirc_fh/lirc_fh_lock
> >
> > however, ir_kbd_i2c's rc_open() callback schedules work whose
> > call-stack looks like:
> > ir_work()
> > ir_key_poll()
> > rc_keydown()
> > ir_do_keydown()
> > ir_lirc_scancode_event() <-- uses lirc_fh/lirc_hf_lock
> >
> > So basically if there is a keydown detection *during* ir_kbd_i2c
> > initialization, its going to oops accessing the uninitialized
> > lirc_fh_lock, I think?
> >
> > Not sure what to do with that info though, even if correct.
> > An 'is registered' check does no good, because the device is marked
> > registered before rc_setup_rx_device() happens. In fact rc_open()
> > already checks this.
> >
> > Hoping someone smarter than me can weigh in here...
>
> I think you have figured it out exactly. That's a highly unlikely race
> condition; in rc_register_device(), in between rc_setup_rx_device()
> and ir_lirc_register() user space must open the input device, and the
> ir_work must be scheduled and execute as well.
>
> The comment on ir_lirc_register() alludes to the same problem, which
> I considered for raw IR devices, but obviously not for scancode devices.
>
> It would be very useful to have this patch tested, I have never seen
> this race condition.
>
> Thanks!
>
> Sean
>
> From d14f6695982b86b383063e74fc6a4febf29248f3 Mon Sep 17 00:00:00 2001
> From: Sean Young <sean@mess.org>
> Date: Thu, 21 Nov 2019 10:10:47 +0000
> Subject: [PATCH] media: rc: ensure lirc is initialized before registering
> input device
>
> Once rc_open is called on the input device, lirc events can be delivered.
> Ensure lirc is ready to do so else we might get this:
>
> Registered IR keymap rc-hauppauge
> rc rc0: Hauppauge WinTV PVR-350 as
> /devices/pci0000:00/0000:00:1e.0/0000:04:00.0/i2c-0/0-0018/rc/rc0
> input: Hauppauge WinTV PVR-350 as
> /devices/pci0000:00/0000:00:1e.0/0000:04:00.0/i2c-0/0-0018/rc/rc0/input9
> BUG: kernel NULL pointer dereference, address: 0000000000000038
> PGD 0 P4D 0
> Oops: 0000 [#1] SMP PTI
> CPU: 1 PID: 17 Comm: kworker/1:0 Not tainted 5.3.11-300.fc31.x86_64 #1
> Hardware name: /DG43NB, BIOS NBG4310H.86A.0096.2009.0903.1845 09/03/2009
> Workqueue: events ir_work [ir_kbd_i2c]
> RIP: 0010:ir_lirc_scancode_event+0x3d/0xb0
> Code: a6 b4 07 00 00 49 81 c6 b8 07 00 00 55 53 e8 ba a7 9d ff 4c 89
> e7 49 89 45 00 e8 5e 7a 25 00 49 8b 1e 48 89 c5 4c 39 f3 74 58 <8b> 43
> 38 8b 53 40 89 c1 2b 4b 3c 39 ca 72 41 21 d0 49 8b 7d 00 49
> RSP: 0018:ffffaae2000b3d88 EFLAGS: 00010017
> RAX: 0000000000000002 RBX: 0000000000000000 RCX: 0000000000000019
> RDX: 0000000000000001 RSI: 006e801b1f26ce6a RDI: ffff9e39797c37b4
> RBP: 0000000000000002 R08: 0000000000000001 R09: 0000000000000001
> R10: 0000000000000001 R11: 0000000000000001 R12: ffff9e39797c37b4
> R13: ffffaae2000b3db8 R14: ffff9e39797c37b8 R15: ffff9e39797c33d8
> FS: 0000000000000000(0000) GS:ffff9e397b680000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000000000038 CR3: 0000000035844000 CR4: 00000000000006e0
> Call Trace:
> ir_do_keydown+0x8e/0x2b0
> rc_keydown+0x52/0xc0
> ir_work+0xb8/0x130 [ir_kbd_i2c]
> process_one_work+0x19d/0x340
> worker_thread+0x50/0x3b0
> kthread+0xfb/0x130
> ? process_one_work+0x340/0x340
> ? kthread_park+0x80/0x80
> ret_from_fork+0x35/0x40
> Modules linked in: rc_hauppauge tuner msp3400 saa7127 saa7115 ivtv(+)
> tveeprom cx2341x v4l2_common videodev mc i2c_algo_bit ir_kbd_i2c
> ip_tables firewire_ohci e1000e serio_raw firewire_core ata_generic
> crc_itu_t pata_acpi pata_jmicron fuse
> CR2: 0000000000000038
> ---[ end trace c67c2697a99fa74b ]---
> RIP: 0010:ir_lirc_scancode_event+0x3d/0xb0
> Code: a6 b4 07 00 00 49 81 c6 b8 07 00 00 55 53 e8 ba a7 9d ff 4c 89
> e7 49 89 45 00 e8 5e 7a 25 00 49 8b 1e 48 89 c5 4c 39 f3 74 58 <8b> 43
> 38 8b 53 40 89 c1 2b 4b 3c 39 ca 72 41 21 d0 49 8b 7d 00 49
> RSP: 0018:ffffaae2000b3d88 EFLAGS: 00010017
> RAX: 0000000000000002 RBX: 0000000000000000 RCX: 0000000000000019
> RDX: 0000000000000001 RSI: 006e801b1f26ce6a RDI: ffff9e39797c37b4
> RBP: 0000000000000002 R08: 0000000000000001 R09: 0000000000000001
> R10: 0000000000000001 R11: 0000000000000001 R12: ffff9e39797c37b4
> R13: ffffaae2000b3db8 R14: ffff9e39797c37b8 R15: ffff9e39797c33d8
> FS: 0000000000000000(0000) GS:ffff9e397b680000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000000000038 CR3: 0000000035844000 CR4: 00000000000006e0
> rc rc0: lirc_dev: driver ir_kbd_i2c registered at minor = 0, scancode
> receiver, no transmitter
> tuner-simple 0-0061: creating new instance
> tuner-simple 0-0061: type set to 2 (Philips NTSC (FI1236,FM1236 and
> compatibles))
> ivtv0: Registered device video0 for encoder MPG (4096 kB)
> ivtv0: Registered device video32 for encoder YUV (2048 kB)
> ivtv0: Registered device vbi0 for encoder VBI (1024 kB)
> ivtv0: Registered device video24 for encoder PCM (320 kB)
> ivtv0: Registered device radio0 for encoder radio
> ivtv0: Registered device video16 for decoder MPG (1024 kB)
> ivtv0: Registered device vbi8 for decoder VBI (64 kB)
> ivtv0: Registered device vbi16 for decoder VOUT
> ivtv0: Registered device video48 for decoder YUV (1024 kB)
>
> Cc: stable@vger.kernel.org
> Reported-by: Nick French <nickfrench@gmail.com>
> Signed-off-by: Sean Young <sean@mess.org>
> ---
> drivers/media/rc/rc-main.c | 20 ++++++++++----------
> 1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
> index 7741151606ef..f29def4f35d8 100644
> --- a/drivers/media/rc/rc-main.c
> +++ b/drivers/media/rc/rc-main.c
> @@ -1891,23 +1891,23 @@ int rc_register_device(struct rc_dev *dev)
>
> dev->registered = true;
>
> - if (dev->driver_type != RC_DRIVER_IR_RAW_TX) {
> - rc = rc_setup_rx_device(dev);
> - if (rc)
> - goto out_dev;
> - }
> -
> /* Ensure that the lirc kfifo is setup before we start the thread */
> if (dev->allowed_protocols != RC_PROTO_BIT_CEC) {
> rc = ir_lirc_register(dev);
> if (rc < 0)
> - goto out_rx;
> + goto out_dev;
> + }
> +
> + if (dev->driver_type != RC_DRIVER_IR_RAW_TX) {
> + rc = rc_setup_rx_device(dev);
> + if (rc)
> + goto out_lirc;
> }
>
> if (dev->driver_type == RC_DRIVER_IR_RAW) {
> rc = ir_raw_event_register(dev);
> if (rc < 0)
> - goto out_lirc;
> + goto out_rx;
> }
>
> dev_dbg(&dev->dev, "Registered rc%u (driver: %s)\n", dev->minor,
> @@ -1915,11 +1915,11 @@ int rc_register_device(struct rc_dev *dev)
>
> return 0;
>
> +out_rx:
> + rc_free_rx_device(dev);
> out_lirc:
> if (dev->allowed_protocols != RC_PROTO_BIT_CEC)
> ir_lirc_unregister(dev);
> -out_rx:
> - rc_free_rx_device(dev);
> out_dev:
> device_del(&dev->dev);
> out_rx_free:
> --
> 2.23.0
>
(re-sending to list)
Hey Sean,
Thanks for the amazingly quick response.
I built that patch and did a few reboots. Everything remains
functional in my setup (1 pvr350 + 1 pchdtv5500).
Obviously I can't verify the intermittent oopses are gone since I
cannot intentionally recreate them, but your patch certainly passes
the eyeball test.
Tested-by: Nick French <nickfrench@gmail.com>
On Thu, Nov 21, 2019 at 4:24 AM Sean Young <sean@mess.org> wrote:
>
> Hi Nick,
>
> On Thu, Nov 21, 2019 at 12:44:42AM -0600, Nick French wrote:
> > On Mon, Nov 18, 2019 at 10:21 AM Nick French <nickfrench@gmail.com> wrote:
> > > I'm about to start trying to track down an intermittent oops in
> > > ir_kbd_i2c that I've been having for a long time over various recent
> > > kernels. It must be some kind of timing/race condition at startup,
> > > because I only get it about 5% of boots.
> > >
> > > Here's the relevant snippet from the log, if anyone has any smart
> > > ideas let me know. I should have better debugging data within the next
> > > week or so.
> > >
> > > ...
> > > Registered IR keymap rc-hauppauge
> > > rc rc0: Hauppauge WinTV PVR-350 as
> > > /devices/pci0000:00/0000:00:1e.0/0000:04:00.0/i2c-0/0-0018/rc/rc0
> > > input: Hauppauge WinTV PVR-350 as
> > > /devices/pci0000:00/0000:00:1e.0/0000:04:00.0/i2c-0/0-0018/rc/rc0/input9
> > > BUG: kernel NULL pointer dereference, address: 0000000000000038
> > > #PF: supervisor read access in kernel mode
> > > #PF: error_code(0x0000) - not-present page
> > > PGD 0 P4D 0
> > > Oops: 0000 [#1] SMP PTI
> > > CPU: 1 PID: 17 Comm: kworker/1:0 Not tainted 5.3.11-300.fc31.x86_64 #1
> > > Hardware name: /DG43NB, BIOS NBG4310H.86A.0096.2009.0903.1845 09/03/2009
> > > Workqueue: events ir_work [ir_kbd_i2c]
> > > RIP: 0010:ir_lirc_scancode_event+0x3d/0xb0
> > > Code: a6 b4 07 00 00 49 81 c6 b8 07 00 00 55 53 e8 ba a7 9d ff 4c 89
> > > e7 49 89 45 00 e8 5e 7a 25 00 49 8b 1e 48 89 c5 4c 39 f3 74 58 <8b> 43
> > > 38 8b 53 40 89 c1 2b 4b 3c 39 ca 72 41 21 d0 49 8b 7d 00 49
> > > RSP: 0018:ffffaae2000b3d88 EFLAGS: 00010017
> > > RAX: 0000000000000002 RBX: 0000000000000000 RCX: 0000000000000019
> > > RDX: 0000000000000001 RSI: 006e801b1f26ce6a RDI: ffff9e39797c37b4
> > > RBP: 0000000000000002 R08: 0000000000000001 R09: 0000000000000001
> > > R10: 0000000000000001 R11: 0000000000000001 R12: ffff9e39797c37b4
> > > R13: ffffaae2000b3db8 R14: ffff9e39797c37b8 R15: ffff9e39797c33d8
> > > FS: 0000000000000000(0000) GS:ffff9e397b680000(0000) knlGS:0000000000000000
> > > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > CR2: 0000000000000038 CR3: 0000000035844000 CR4: 00000000000006e0
> > > Call Trace:
> > > ir_do_keydown+0x8e/0x2b0
> > > rc_keydown+0x52/0xc0
> > > ir_work+0xb8/0x130 [ir_kbd_i2c]
> > > process_one_work+0x19d/0x340
> > > worker_thread+0x50/0x3b0
> > > kthread+0xfb/0x130
> > > ? process_one_work+0x340/0x340
> > > ? kthread_park+0x80/0x80
> > > ret_from_fork+0x35/0x40
> > > Modules linked in: rc_hauppauge tuner msp3400 saa7127 saa7115 ivtv(+)
> > > tveeprom cx2341x v4l2_common videodev mc i2c_algo_bit ir_kbd_i2c
> > > ip_tables firewire_ohci e1000e serio_raw firewire_core ata_generic
> > > crc_itu_t pata_acpi pata_jmicron fuse
> > > CR2: 0000000000000038
> > > ---[ end trace c67c2697a99fa74b ]---
> > > RIP: 0010:ir_lirc_scancode_event+0x3d/0xb0
> > > Code: a6 b4 07 00 00 49 81 c6 b8 07 00 00 55 53 e8 ba a7 9d ff 4c 89
> > > e7 49 89 45 00 e8 5e 7a 25 00 49 8b 1e 48 89 c5 4c 39 f3 74 58 <8b> 43
> > > 38 8b 53 40 89 c1 2b 4b 3c 39 ca 72 41 21 d0 49 8b 7d 00 49
> > > RSP: 0018:ffffaae2000b3d88 EFLAGS: 00010017
> > > RAX: 0000000000000002 RBX: 0000000000000000 RCX: 0000000000000019
> > > RDX: 0000000000000001 RSI: 006e801b1f26ce6a RDI: ffff9e39797c37b4
> > > RBP: 0000000000000002 R08: 0000000000000001 R09: 0000000000000001
> > > R10: 0000000000000001 R11: 0000000000000001 R12: ffff9e39797c37b4
> > > R13: ffffaae2000b3db8 R14: ffff9e39797c37b8 R15: ffff9e39797c33d8
> > > FS: 0000000000000000(0000) GS:ffff9e397b680000(0000) knlGS:0000000000000000
> > > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > CR2: 0000000000000038 CR3: 0000000035844000 CR4: 00000000000006e0
> > > rc rc0: lirc_dev: driver ir_kbd_i2c registered at minor = 0, scancode
> > > receiver, no transmitter
> > > tuner-simple 0-0061: creating new instance
> > > tuner-simple 0-0061: type set to 2 (Philips NTSC (FI1236,FM1236 and
> > > compatibles))
> > > ivtv0: Registered device video0 for encoder MPG (4096 kB)
> > > ivtv0: Registered device video32 for encoder YUV (2048 kB)
> > > ivtv0: Registered device vbi0 for encoder VBI (1024 kB)
> > > ivtv0: Registered device video24 for encoder PCM (320 kB)
> > > ivtv0: Registered device radio0 for encoder radio
> > > ivtv0: Registered device video16 for decoder MPG (1024 kB)
> > > ivtv0: Registered device vbi8 for decoder VBI (64 kB)
> > > ivtv0: Registered device vbi16 for decoder VOUT
> > > ivtv0: Registered device video48 for decoder YUV (1024 kB)
> > > ...
> > >
> > > - Nick
> >
> > I haven't been able to recreate with a debug build yet, but I think I
> > see the problem:
> >
> > ir_kbd_i2c calls rc_register_device(), where the ordering of calls is this:
> > dev->registered = true;
> > rc_setup_rx_device(dev); <--calls rc_open()
> > ir_lirc_register(dev); <--initializes lirc_fh/lirc_fh_lock
> >
> > however, ir_kbd_i2c's rc_open() callback schedules work whose
> > call-stack looks like:
> > ir_work()
> > ir_key_poll()
> > rc_keydown()
> > ir_do_keydown()
> > ir_lirc_scancode_event() <-- uses lirc_fh/lirc_hf_lock
> >
> > So basically if there is a keydown detection *during* ir_kbd_i2c
> > initialization, its going to oops accessing the uninitialized
> > lirc_fh_lock, I think?
> >
> > Not sure what to do with that info though, even if correct.
> > An 'is registered' check does no good, because the device is marked
> > registered before rc_setup_rx_device() happens. In fact rc_open()
> > already checks this.
> >
> > Hoping someone smarter than me can weigh in here...
>
> I think you have figured it out exactly. That's a highly unlikely race
> condition; in rc_register_device(), in between rc_setup_rx_device()
> and ir_lirc_register() user space must open the input device, and the
> ir_work must be scheduled and execute as well.
>
> The comment on ir_lirc_register() alludes to the same problem, which
> I considered for raw IR devices, but obviously not for scancode devices.
>
> It would be very useful to have this patch tested, I have never seen
> this race condition.
>
> Thanks!
>
> Sean
>
> From d14f6695982b86b383063e74fc6a4febf29248f3 Mon Sep 17 00:00:00 2001
> From: Sean Young <sean@mess.org>
> Date: Thu, 21 Nov 2019 10:10:47 +0000
> Subject: [PATCH] media: rc: ensure lirc is initialized before registering
> input device
>
> Once rc_open is called on the input device, lirc events can be delivered.
> Ensure lirc is ready to do so else we might get this:
>
> Registered IR keymap rc-hauppauge
> rc rc0: Hauppauge WinTV PVR-350 as
> /devices/pci0000:00/0000:00:1e.0/0000:04:00.0/i2c-0/0-0018/rc/rc0
> input: Hauppauge WinTV PVR-350 as
> /devices/pci0000:00/0000:00:1e.0/0000:04:00.0/i2c-0/0-0018/rc/rc0/input9
> BUG: kernel NULL pointer dereference, address: 0000000000000038
> PGD 0 P4D 0
> Oops: 0000 [#1] SMP PTI
> CPU: 1 PID: 17 Comm: kworker/1:0 Not tainted 5.3.11-300.fc31.x86_64 #1
> Hardware name: /DG43NB, BIOS NBG4310H.86A.0096.2009.0903.1845 09/03/2009
> Workqueue: events ir_work [ir_kbd_i2c]
> RIP: 0010:ir_lirc_scancode_event+0x3d/0xb0
> Code: a6 b4 07 00 00 49 81 c6 b8 07 00 00 55 53 e8 ba a7 9d ff 4c 89
> e7 49 89 45 00 e8 5e 7a 25 00 49 8b 1e 48 89 c5 4c 39 f3 74 58 <8b> 43
> 38 8b 53 40 89 c1 2b 4b 3c 39 ca 72 41 21 d0 49 8b 7d 00 49
> RSP: 0018:ffffaae2000b3d88 EFLAGS: 00010017
> RAX: 0000000000000002 RBX: 0000000000000000 RCX: 0000000000000019
> RDX: 0000000000000001 RSI: 006e801b1f26ce6a RDI: ffff9e39797c37b4
> RBP: 0000000000000002 R08: 0000000000000001 R09: 0000000000000001
> R10: 0000000000000001 R11: 0000000000000001 R12: ffff9e39797c37b4
> R13: ffffaae2000b3db8 R14: ffff9e39797c37b8 R15: ffff9e39797c33d8
> FS: 0000000000000000(0000) GS:ffff9e397b680000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000000000038 CR3: 0000000035844000 CR4: 00000000000006e0
> Call Trace:
> ir_do_keydown+0x8e/0x2b0
> rc_keydown+0x52/0xc0
> ir_work+0xb8/0x130 [ir_kbd_i2c]
> process_one_work+0x19d/0x340
> worker_thread+0x50/0x3b0
> kthread+0xfb/0x130
> ? process_one_work+0x340/0x340
> ? kthread_park+0x80/0x80
> ret_from_fork+0x35/0x40
> Modules linked in: rc_hauppauge tuner msp3400 saa7127 saa7115 ivtv(+)
> tveeprom cx2341x v4l2_common videodev mc i2c_algo_bit ir_kbd_i2c
> ip_tables firewire_ohci e1000e serio_raw firewire_core ata_generic
> crc_itu_t pata_acpi pata_jmicron fuse
> CR2: 0000000000000038
> ---[ end trace c67c2697a99fa74b ]---
> RIP: 0010:ir_lirc_scancode_event+0x3d/0xb0
> Code: a6 b4 07 00 00 49 81 c6 b8 07 00 00 55 53 e8 ba a7 9d ff 4c 89
> e7 49 89 45 00 e8 5e 7a 25 00 49 8b 1e 48 89 c5 4c 39 f3 74 58 <8b> 43
> 38 8b 53 40 89 c1 2b 4b 3c 39 ca 72 41 21 d0 49 8b 7d 00 49
> RSP: 0018:ffffaae2000b3d88 EFLAGS: 00010017
> RAX: 0000000000000002 RBX: 0000000000000000 RCX: 0000000000000019
> RDX: 0000000000000001 RSI: 006e801b1f26ce6a RDI: ffff9e39797c37b4
> RBP: 0000000000000002 R08: 0000000000000001 R09: 0000000000000001
> R10: 0000000000000001 R11: 0000000000000001 R12: ffff9e39797c37b4
> R13: ffffaae2000b3db8 R14: ffff9e39797c37b8 R15: ffff9e39797c33d8
> FS: 0000000000000000(0000) GS:ffff9e397b680000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000000000038 CR3: 0000000035844000 CR4: 00000000000006e0
> rc rc0: lirc_dev: driver ir_kbd_i2c registered at minor = 0, scancode
> receiver, no transmitter
> tuner-simple 0-0061: creating new instance
> tuner-simple 0-0061: type set to 2 (Philips NTSC (FI1236,FM1236 and
> compatibles))
> ivtv0: Registered device video0 for encoder MPG (4096 kB)
> ivtv0: Registered device video32 for encoder YUV (2048 kB)
> ivtv0: Registered device vbi0 for encoder VBI (1024 kB)
> ivtv0: Registered device video24 for encoder PCM (320 kB)
> ivtv0: Registered device radio0 for encoder radio
> ivtv0: Registered device video16 for decoder MPG (1024 kB)
> ivtv0: Registered device vbi8 for decoder VBI (64 kB)
> ivtv0: Registered device vbi16 for decoder VOUT
> ivtv0: Registered device video48 for decoder YUV (1024 kB)
>
> Cc: stable@vger.kernel.org
> Reported-by: Nick French <nickfrench@gmail.com>
> Signed-off-by: Sean Young <sean@mess.org>
> ---
> drivers/media/rc/rc-main.c | 20 ++++++++++----------
> 1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
> index 7741151606ef..f29def4f35d8 100644
> --- a/drivers/media/rc/rc-main.c
> +++ b/drivers/media/rc/rc-main.c
> @@ -1891,23 +1891,23 @@ int rc_register_device(struct rc_dev *dev)
>
> dev->registered = true;
>
> - if (dev->driver_type != RC_DRIVER_IR_RAW_TX) {
> - rc = rc_setup_rx_device(dev);
> - if (rc)
> - goto out_dev;
> - }
> -
> /* Ensure that the lirc kfifo is setup before we start the thread */
> if (dev->allowed_protocols != RC_PROTO_BIT_CEC) {
> rc = ir_lirc_register(dev);
> if (rc < 0)
> - goto out_rx;
> + goto out_dev;
> + }
> +
> + if (dev->driver_type != RC_DRIVER_IR_RAW_TX) {
> + rc = rc_setup_rx_device(dev);
> + if (rc)
> + goto out_lirc;
> }
>
> if (dev->driver_type == RC_DRIVER_IR_RAW) {
> rc = ir_raw_event_register(dev);
> if (rc < 0)
> - goto out_lirc;
> + goto out_rx;
> }
>
> dev_dbg(&dev->dev, "Registered rc%u (driver: %s)\n", dev->minor,
> @@ -1915,11 +1915,11 @@ int rc_register_device(struct rc_dev *dev)
>
> return 0;
>
> +out_rx:
> + rc_free_rx_device(dev);
> out_lirc:
> if (dev->allowed_protocols != RC_PROTO_BIT_CEC)
> ir_lirc_unregister(dev);
> -out_rx:
> - rc_free_rx_device(dev);
> out_dev:
> device_del(&dev->dev);
> out_rx_free:
> --
> 2.23.0
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: ir_kbd_i2c oops
2019-11-21 14:19 ` Nick French
@ 2019-12-10 19:26 ` Nick French
0 siblings, 0 replies; 5+ messages in thread
From: Nick French @ 2019-12-10 19:26 UTC (permalink / raw)
To: Sean Young; +Cc: linux-media
On Thu, Nov 21, 2019 at 8:19 AM Nick French <nickfrench@gmail.com> wrote:
>
> On Thu, Nov 21, 2019 at 4:24 AM Sean Young <sean@mess.org> wrote:
> >
> > Hi Nick,
> >
> > On Thu, Nov 21, 2019 at 12:44:42AM -0600, Nick French wrote:
> > > On Mon, Nov 18, 2019 at 10:21 AM Nick French <nickfrench@gmail.com> wrote:
> > > > I'm about to start trying to track down an intermittent oops in
> > > > ir_kbd_i2c that I've been having for a long time over various recent
> > > > kernels. It must be some kind of timing/race condition at startup,
> > > > because I only get it about 5% of boots.
> > > >
> > > > Here's the relevant snippet from the log, if anyone has any smart
> > > > ideas let me know. I should have better debugging data within the next
> > > > week or so.
> > > >
> > > > ...
> > > > Registered IR keymap rc-hauppauge
> > > > rc rc0: Hauppauge WinTV PVR-350 as
> > > > /devices/pci0000:00/0000:00:1e.0/0000:04:00.0/i2c-0/0-0018/rc/rc0
> > > > input: Hauppauge WinTV PVR-350 as
> > > > /devices/pci0000:00/0000:00:1e.0/0000:04:00.0/i2c-0/0-0018/rc/rc0/input9
> > > > BUG: kernel NULL pointer dereference, address: 0000000000000038
> > > > #PF: supervisor read access in kernel mode
> > > > #PF: error_code(0x0000) - not-present page
> > > > PGD 0 P4D 0
> > > > Oops: 0000 [#1] SMP PTI
> > > > CPU: 1 PID: 17 Comm: kworker/1:0 Not tainted 5.3.11-300.fc31.x86_64 #1
> > > > Hardware name: /DG43NB, BIOS NBG4310H.86A.0096.2009.0903.1845 09/03/2009
> > > > Workqueue: events ir_work [ir_kbd_i2c]
> > > > RIP: 0010:ir_lirc_scancode_event+0x3d/0xb0
> > > > Code: a6 b4 07 00 00 49 81 c6 b8 07 00 00 55 53 e8 ba a7 9d ff 4c 89
> > > > e7 49 89 45 00 e8 5e 7a 25 00 49 8b 1e 48 89 c5 4c 39 f3 74 58 <8b> 43
> > > > 38 8b 53 40 89 c1 2b 4b 3c 39 ca 72 41 21 d0 49 8b 7d 00 49
> > > > RSP: 0018:ffffaae2000b3d88 EFLAGS: 00010017
> > > > RAX: 0000000000000002 RBX: 0000000000000000 RCX: 0000000000000019
> > > > RDX: 0000000000000001 RSI: 006e801b1f26ce6a RDI: ffff9e39797c37b4
> > > > RBP: 0000000000000002 R08: 0000000000000001 R09: 0000000000000001
> > > > R10: 0000000000000001 R11: 0000000000000001 R12: ffff9e39797c37b4
> > > > R13: ffffaae2000b3db8 R14: ffff9e39797c37b8 R15: ffff9e39797c33d8
> > > > FS: 0000000000000000(0000) GS:ffff9e397b680000(0000) knlGS:0000000000000000
> > > > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > > CR2: 0000000000000038 CR3: 0000000035844000 CR4: 00000000000006e0
> > > > Call Trace:
> > > > ir_do_keydown+0x8e/0x2b0
> > > > rc_keydown+0x52/0xc0
> > > > ir_work+0xb8/0x130 [ir_kbd_i2c]
> > > > process_one_work+0x19d/0x340
> > > > worker_thread+0x50/0x3b0
> > > > kthread+0xfb/0x130
> > > > ? process_one_work+0x340/0x340
> > > > ? kthread_park+0x80/0x80
> > > > ret_from_fork+0x35/0x40
> > > > Modules linked in: rc_hauppauge tuner msp3400 saa7127 saa7115 ivtv(+)
> > > > tveeprom cx2341x v4l2_common videodev mc i2c_algo_bit ir_kbd_i2c
> > > > ip_tables firewire_ohci e1000e serio_raw firewire_core ata_generic
> > > > crc_itu_t pata_acpi pata_jmicron fuse
> > > > CR2: 0000000000000038
> > > > ---[ end trace c67c2697a99fa74b ]---
> > > > RIP: 0010:ir_lirc_scancode_event+0x3d/0xb0
> > > > Code: a6 b4 07 00 00 49 81 c6 b8 07 00 00 55 53 e8 ba a7 9d ff 4c 89
> > > > e7 49 89 45 00 e8 5e 7a 25 00 49 8b 1e 48 89 c5 4c 39 f3 74 58 <8b> 43
> > > > 38 8b 53 40 89 c1 2b 4b 3c 39 ca 72 41 21 d0 49 8b 7d 00 49
> > > > RSP: 0018:ffffaae2000b3d88 EFLAGS: 00010017
> > > > RAX: 0000000000000002 RBX: 0000000000000000 RCX: 0000000000000019
> > > > RDX: 0000000000000001 RSI: 006e801b1f26ce6a RDI: ffff9e39797c37b4
> > > > RBP: 0000000000000002 R08: 0000000000000001 R09: 0000000000000001
> > > > R10: 0000000000000001 R11: 0000000000000001 R12: ffff9e39797c37b4
> > > > R13: ffffaae2000b3db8 R14: ffff9e39797c37b8 R15: ffff9e39797c33d8
> > > > FS: 0000000000000000(0000) GS:ffff9e397b680000(0000) knlGS:0000000000000000
> > > > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > > CR2: 0000000000000038 CR3: 0000000035844000 CR4: 00000000000006e0
> > > > rc rc0: lirc_dev: driver ir_kbd_i2c registered at minor = 0, scancode
> > > > receiver, no transmitter
> > > > tuner-simple 0-0061: creating new instance
> > > > tuner-simple 0-0061: type set to 2 (Philips NTSC (FI1236,FM1236 and
> > > > compatibles))
> > > > ivtv0: Registered device video0 for encoder MPG (4096 kB)
> > > > ivtv0: Registered device video32 for encoder YUV (2048 kB)
> > > > ivtv0: Registered device vbi0 for encoder VBI (1024 kB)
> > > > ivtv0: Registered device video24 for encoder PCM (320 kB)
> > > > ivtv0: Registered device radio0 for encoder radio
> > > > ivtv0: Registered device video16 for decoder MPG (1024 kB)
> > > > ivtv0: Registered device vbi8 for decoder VBI (64 kB)
> > > > ivtv0: Registered device vbi16 for decoder VOUT
> > > > ivtv0: Registered device video48 for decoder YUV (1024 kB)
> > > > ...
> > > >
> > > > - Nick
> > >
> > > I haven't been able to recreate with a debug build yet, but I think I
> > > see the problem:
> > >
> > > ir_kbd_i2c calls rc_register_device(), where the ordering of calls is this:
> > > dev->registered = true;
> > > rc_setup_rx_device(dev); <--calls rc_open()
> > > ir_lirc_register(dev); <--initializes lirc_fh/lirc_fh_lock
> > >
> > > however, ir_kbd_i2c's rc_open() callback schedules work whose
> > > call-stack looks like:
> > > ir_work()
> > > ir_key_poll()
> > > rc_keydown()
> > > ir_do_keydown()
> > > ir_lirc_scancode_event() <-- uses lirc_fh/lirc_hf_lock
> > >
> > > So basically if there is a keydown detection *during* ir_kbd_i2c
> > > initialization, its going to oops accessing the uninitialized
> > > lirc_fh_lock, I think?
> > >
> > > Not sure what to do with that info though, even if correct.
> > > An 'is registered' check does no good, because the device is marked
> > > registered before rc_setup_rx_device() happens. In fact rc_open()
> > > already checks this.
> > >
> > > Hoping someone smarter than me can weigh in here...
> >
> > I think you have figured it out exactly. That's a highly unlikely race
> > condition; in rc_register_device(), in between rc_setup_rx_device()
> > and ir_lirc_register() user space must open the input device, and the
> > ir_work must be scheduled and execute as well.
> >
> > The comment on ir_lirc_register() alludes to the same problem, which
> > I considered for raw IR devices, but obviously not for scancode devices.
> >
> > It would be very useful to have this patch tested, I have never seen
> > this race condition.
> >
> > Thanks!
> >
> > Sean
> >
> > From d14f6695982b86b383063e74fc6a4febf29248f3 Mon Sep 17 00:00:00 2001
> > From: Sean Young <sean@mess.org>
> > Date: Thu, 21 Nov 2019 10:10:47 +0000
> > Subject: [PATCH] media: rc: ensure lirc is initialized before registering
> > input device
> >
> > Once rc_open is called on the input device, lirc events can be delivered.
> > Ensure lirc is ready to do so else we might get this:
> >
> > Registered IR keymap rc-hauppauge
> > rc rc0: Hauppauge WinTV PVR-350 as
> > /devices/pci0000:00/0000:00:1e.0/0000:04:00.0/i2c-0/0-0018/rc/rc0
> > input: Hauppauge WinTV PVR-350 as
> > /devices/pci0000:00/0000:00:1e.0/0000:04:00.0/i2c-0/0-0018/rc/rc0/input9
> > BUG: kernel NULL pointer dereference, address: 0000000000000038
> > PGD 0 P4D 0
> > Oops: 0000 [#1] SMP PTI
> > CPU: 1 PID: 17 Comm: kworker/1:0 Not tainted 5.3.11-300.fc31.x86_64 #1
> > Hardware name: /DG43NB, BIOS NBG4310H.86A.0096.2009.0903.1845 09/03/2009
> > Workqueue: events ir_work [ir_kbd_i2c]
> > RIP: 0010:ir_lirc_scancode_event+0x3d/0xb0
> > Code: a6 b4 07 00 00 49 81 c6 b8 07 00 00 55 53 e8 ba a7 9d ff 4c 89
> > e7 49 89 45 00 e8 5e 7a 25 00 49 8b 1e 48 89 c5 4c 39 f3 74 58 <8b> 43
> > 38 8b 53 40 89 c1 2b 4b 3c 39 ca 72 41 21 d0 49 8b 7d 00 49
> > RSP: 0018:ffffaae2000b3d88 EFLAGS: 00010017
> > RAX: 0000000000000002 RBX: 0000000000000000 RCX: 0000000000000019
> > RDX: 0000000000000001 RSI: 006e801b1f26ce6a RDI: ffff9e39797c37b4
> > RBP: 0000000000000002 R08: 0000000000000001 R09: 0000000000000001
> > R10: 0000000000000001 R11: 0000000000000001 R12: ffff9e39797c37b4
> > R13: ffffaae2000b3db8 R14: ffff9e39797c37b8 R15: ffff9e39797c33d8
> > FS: 0000000000000000(0000) GS:ffff9e397b680000(0000) knlGS:0000000000000000
> > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > CR2: 0000000000000038 CR3: 0000000035844000 CR4: 00000000000006e0
> > Call Trace:
> > ir_do_keydown+0x8e/0x2b0
> > rc_keydown+0x52/0xc0
> > ir_work+0xb8/0x130 [ir_kbd_i2c]
> > process_one_work+0x19d/0x340
> > worker_thread+0x50/0x3b0
> > kthread+0xfb/0x130
> > ? process_one_work+0x340/0x340
> > ? kthread_park+0x80/0x80
> > ret_from_fork+0x35/0x40
> > Modules linked in: rc_hauppauge tuner msp3400 saa7127 saa7115 ivtv(+)
> > tveeprom cx2341x v4l2_common videodev mc i2c_algo_bit ir_kbd_i2c
> > ip_tables firewire_ohci e1000e serio_raw firewire_core ata_generic
> > crc_itu_t pata_acpi pata_jmicron fuse
> > CR2: 0000000000000038
> > ---[ end trace c67c2697a99fa74b ]---
> > RIP: 0010:ir_lirc_scancode_event+0x3d/0xb0
> > Code: a6 b4 07 00 00 49 81 c6 b8 07 00 00 55 53 e8 ba a7 9d ff 4c 89
> > e7 49 89 45 00 e8 5e 7a 25 00 49 8b 1e 48 89 c5 4c 39 f3 74 58 <8b> 43
> > 38 8b 53 40 89 c1 2b 4b 3c 39 ca 72 41 21 d0 49 8b 7d 00 49
> > RSP: 0018:ffffaae2000b3d88 EFLAGS: 00010017
> > RAX: 0000000000000002 RBX: 0000000000000000 RCX: 0000000000000019
> > RDX: 0000000000000001 RSI: 006e801b1f26ce6a RDI: ffff9e39797c37b4
> > RBP: 0000000000000002 R08: 0000000000000001 R09: 0000000000000001
> > R10: 0000000000000001 R11: 0000000000000001 R12: ffff9e39797c37b4
> > R13: ffffaae2000b3db8 R14: ffff9e39797c37b8 R15: ffff9e39797c33d8
> > FS: 0000000000000000(0000) GS:ffff9e397b680000(0000) knlGS:0000000000000000
> > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > CR2: 0000000000000038 CR3: 0000000035844000 CR4: 00000000000006e0
> > rc rc0: lirc_dev: driver ir_kbd_i2c registered at minor = 0, scancode
> > receiver, no transmitter
> > tuner-simple 0-0061: creating new instance
> > tuner-simple 0-0061: type set to 2 (Philips NTSC (FI1236,FM1236 and
> > compatibles))
> > ivtv0: Registered device video0 for encoder MPG (4096 kB)
> > ivtv0: Registered device video32 for encoder YUV (2048 kB)
> > ivtv0: Registered device vbi0 for encoder VBI (1024 kB)
> > ivtv0: Registered device video24 for encoder PCM (320 kB)
> > ivtv0: Registered device radio0 for encoder radio
> > ivtv0: Registered device video16 for decoder MPG (1024 kB)
> > ivtv0: Registered device vbi8 for decoder VBI (64 kB)
> > ivtv0: Registered device vbi16 for decoder VOUT
> > ivtv0: Registered device video48 for decoder YUV (1024 kB)
> >
> > Cc: stable@vger.kernel.org
> > Reported-by: Nick French <nickfrench@gmail.com>
> > Signed-off-by: Sean Young <sean@mess.org>
> > ---
> > drivers/media/rc/rc-main.c | 20 ++++++++++----------
> > 1 file changed, 10 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
> > index 7741151606ef..f29def4f35d8 100644
> > --- a/drivers/media/rc/rc-main.c
> > +++ b/drivers/media/rc/rc-main.c
> > @@ -1891,23 +1891,23 @@ int rc_register_device(struct rc_dev *dev)
> >
> > dev->registered = true;
> >
> > - if (dev->driver_type != RC_DRIVER_IR_RAW_TX) {
> > - rc = rc_setup_rx_device(dev);
> > - if (rc)
> > - goto out_dev;
> > - }
> > -
> > /* Ensure that the lirc kfifo is setup before we start the thread */
> > if (dev->allowed_protocols != RC_PROTO_BIT_CEC) {
> > rc = ir_lirc_register(dev);
> > if (rc < 0)
> > - goto out_rx;
> > + goto out_dev;
> > + }
> > +
> > + if (dev->driver_type != RC_DRIVER_IR_RAW_TX) {
> > + rc = rc_setup_rx_device(dev);
> > + if (rc)
> > + goto out_lirc;
> > }
> >
> > if (dev->driver_type == RC_DRIVER_IR_RAW) {
> > rc = ir_raw_event_register(dev);
> > if (rc < 0)
> > - goto out_lirc;
> > + goto out_rx;
> > }
> >
> > dev_dbg(&dev->dev, "Registered rc%u (driver: %s)\n", dev->minor,
> > @@ -1915,11 +1915,11 @@ int rc_register_device(struct rc_dev *dev)
> >
> > return 0;
> >
> > +out_rx:
> > + rc_free_rx_device(dev);
> > out_lirc:
> > if (dev->allowed_protocols != RC_PROTO_BIT_CEC)
> > ir_lirc_unregister(dev);
> > -out_rx:
> > - rc_free_rx_device(dev);
> > out_dev:
> > device_del(&dev->dev);
> > out_rx_free:
> > --
> > 2.23.0
> >
>
> (re-sending to list)
>
> Hey Sean,
>
> Thanks for the amazingly quick response.
>
> I built that patch and did a few reboots. Everything remains
> functional in my setup (1 pvr350 + 1 pchdtv5500).
> Obviously I can't verify the intermittent oopses are gone since I
> cannot intentionally recreate them, but your patch certainly passes
> the eyeball test.
>
> Tested-by: Nick French <nickfrench@gmail.com>
Sean,
A quick update: still no oops after ~2.5 weeks of running with above patch.
The ~2.5 weeks is a significant amount of time without an oops
compared to previous unpatched kernels at ~3 oops/week (the box powers
on/off multiple times per day so it gets lots of bootup-timing
samples), so I'm personally convinced your patch is indeed a stability
improvement.
- Nick
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-12-10 19:26 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-18 16:21 ir_kbd_i2c oops Nick French
2019-11-21 6:44 ` Nick French
2019-11-21 10:24 ` Sean Young
2019-11-21 14:19 ` Nick French
2019-12-10 19:26 ` Nick French
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).