Bug 198675 - Unable to handle kernel NULL pointer dereference with pvrusb2 driver
Summary: Unable to handle kernel NULL pointer dereference with pvrusb2 driver
Status: NEW
Alias: None
Product: Drivers
Classification: Unclassified
Component: USB (show other bugs)
Hardware: ARM Linux
: P1 normal
Assignee: Greg Kroah-Hartman
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-02-05 03:16 UTC by chris
Modified: 2018-02-05 21:14 UTC (History)
2 users (show)

See Also:
Kernel Version: 4.9.75
Tree: Mainline
Regression: No


Attachments
Patch that replaces the NULL (680 bytes, text/plain)
2018-02-05 03:16 UTC, chris
Details
Failed hunk from patch (706 bytes, text/plain)
2018-02-05 19:23 UTC, chris
Details
backported 7cda4c5bae46ffca3abeadc4c1882d9325ee3102 (6.07 KB, patch)
2018-02-05 20:24 UTC, Pablo Sebastian Greco
Details | Diff

Description chris 2018-02-05 03:16:09 UTC
Created attachment 273997 [details]
Patch that replaces the NULL

Seeing the oops below with the pvrusb2 driver:

[SNIP]

Jan 30 04:43:59 bananapi kernel: pvrusb2: Attached sub-driver tuner
Jan 30 04:44:01 bananapi kernel: cx25840 2-0044: loaded v4l-cx25840.fw firmware (16382 bytes) 
Jan 30 04:44:02 bananapi kernel: Unable to handle kernel NULL pointer dereference at virtual address 00000018
Jan 30 04:44:02 bananapi kernel: pgd = c0204000
Jan 30 04:44:02 bananapi kernel: [00000018] *pgd=00000000

Message from syslogd@bananapi at Jan 30 04:44:03 ...
 kernel:Internal error: Oops: 5 [#1] SMP ARM
Jan 30 04:44:03 bananapi kernel: Internal error: Oops: 5 [#1] SMP ARM 

[SNIP]

Jan 30 04:44:03 bananapi kernel: CPU: 1 PID: 824 Comm: pvrusb2-context Tainted: G         C  E 4.9.75-204.el7.centos.armv7hl #1
Jan 30 04:44:03 bananapi kernel: Hardware name: Allwinner sun7i (A20) Family
Jan 30 04:44:03 bananapi kernel: task: c7d48000 task.stack: c7f00000
Jan 30 04:44:03 bananapi kernel: PC is at tveeprom_hauppauge_analog+0x6d8/0x9c0 [tveeprom]
Jan 30 04:44:03 bananapi kernel: LR is at tveeprom_hauppauge_analog+0x58/0x9c0 [tveeprom]
Jan 30 04:44:03 bananapi kernel: pc : [<bf4746d8>]    lr : [<bf474058>]    psr: 60000013#012sp : c7f01d88  ip : bf476c2b  fp : 000000f0
Jan 30 04:44:03 bananapi kernel: r10: 00000000  r9 : 00000025  r8 : bf4769fe
Jan 30 04:44:03 bananapi kernel: r7 : 0000009b  r6 : 00000000  r5 : bf475d5c  r4 : c7f01e4c
Jan 30 04:44:03 bananapi kernel: r3 : 00000008  r2 : 00000000  r1 : 00012567  r0 : 00000000
Jan 30 04:44:03 bananapi kernel: Flags: nZCv  IRQs on  FIQs on Mode SVC_32  ISA ARM  Segment none
Jan 30 04:44:03 bananapi kernel: Control: 10c5387d  Table: 47d2806a  DAC: 00000051 

[SNIP]


Problem is being caused by NULL being passed in : drivers/media/usb/pvrusb2/pvrusb2-eeprom.c:

tveeprom_hauppauge_analog(NULL, &tvdata, eeprom);

I have tested the attached patch, which resolves the issue by replacing the NULL.

Thanks.
Comment 2 Arnd Bergmann 2018-02-05 11:38:45 UTC
I think I see the problem: When I did commit 6830733d53a4 ("[media] pvrusb2: reduce stack usage pvr2_eeprom_analyze()") for v4.11, I verified that it was correct. However, that analysis depended on Mauro's commit 6037b3ca28f4 ("[media] tveeprom: print log messages using pr_foo()"), which was merged in 4.10.

When my patch got backported to v4.9, that assumption was broken. Can you verify after applying commit 6037b3ca28f4? Backporting that to stable as well should be straightforward, if Greg doesn't think it's excessively large.
Comment 3 Pablo Sebastian Greco 2018-02-05 12:07:06 UTC
Chris has already tested that reverting your patch works by rebuilding only the module, so that part is confirmed.
I don't have the hardware to test this, so it'll have to be Chris.

Chris, can you try what Arnd said? Instead of reverting the first patch, applying the last two?
Comment 4 chris 2018-02-05 19:22:20 UTC
I have tested both commits to tveeprom.c, the first patch failed  (7cda4c5bae46ffca3abeadc4c1882d9325ee3102):

patching file tveeprom.c
Hunk #3 FAILED at 496.
Hunk #4 succeeded at 648 (offset 1 line).
Hunk #5 succeeded at 668 (offset 1 line).
Hunk #6 succeeded at 704 (offset 1 line).
Hunk #7 succeeded at 759 (offset 1 line).
1 out of 7 hunks FAILED -- saving rejects to file tveeprom.c.rej

The failure was because:

- tveeprom_warn("Encountered bad packet header [%02x]. Corrupt or not a Hauppauge eeprom.\n",

Should be the below (it is split in to two lines, not a single line):

- tveeprom_warn("Encountered bad packet header [%02x]. "
- "Corrupt or not a Hauppauge eeprom.\n",

I have attached the .rej file for reference of the failed hunk.

However, after making the manual edits above, and re-compiling just tveeprom.c, the changes seem to be working fine with this kernel.

Please let me know if I can be of further help.  Thank you.
Comment 5 chris 2018-02-05 19:23:35 UTC
Created attachment 274007 [details]
Failed hunk from patch
Comment 6 Pablo Sebastian Greco 2018-02-05 20:24:39 UTC
Created attachment 274009 [details]
backported 7cda4c5bae46ffca3abeadc4c1882d9325ee3102

Chris, please use this patch as the first link (7cda4c5bae46ffca3abeadc4c1882d9325ee3102), the second one applies cleanly
Comment 7 chris 2018-02-05 20:44:44 UTC
Pablo,
Confirmed... the patch works here too, and second one as well.
Comment 8 Pablo Sebastian Greco 2018-02-05 21:14:55 UTC
I forgot this commit which should have been applied first (https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/commit/?id=2a3edc5d55c23ee7e2c7fa662ac76cc41863c7a4), which is why that hunk failed.

Note You need to log in before you can comment on or make changes to this bug.