From: "Sven Zühlsdorf" <sven.zuehlsdorf@vigem.de>
To: Rishi Gupta <gupt21@gmail.com>
Cc: linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: NULL pointer dereferences in hid-mcp2221
Date: Tue, 25 Oct 2022 09:39:48 +0200 (CEST) [thread overview]
Message-ID: <79152feb-bcbc-9e3e-e776-13170ae4ef40@vigem.de> (raw)
[-- Attachment #1: Type: text/plain, Size: 11029 bytes --]
Hi,
I've run into two NULL pointer dereferences when loading the MCP2221 driver.
Initially I observed them running the kernel used by yocto kirkstone
(currently 5.15.68) but can reproduce them with a vanilla 6.1-rc1 as well.
All line numbers below are for hid-mcp2221.c, taken from 6.1-rc1.
The first one was easy to identify, in mcp2221_probe line 874 `hdev->hidraw`
was NULL since I compiled the kernel without CONFIG_HIDRAW enabled. Should
CONFIG_HID_MCP2221 perhaps depend on or imply CONFIG_HIDRAW?
For the second one however, I'm stuck. The dereference happens at the top of
mcp_smbus_xfer since i2c_get_adapdata in line 307 returned NULL. Looking at
the call trace (see [dmesg output] below), mcp_smbus_xfer gets called
indirectly from mcp2221_probe via i2c_add_adapter in line 876, directly
before a call to i2c_set_adapdata. Since I couldn't identify another call to
i2c_set_adapdata or a write to `mcp->adapter.dev.driver_data` that could
potentially have initialized that field, I attempted to swap the order of
calling i2c_set_adapdata and i2c_add_adapter, see [attempted fix] below. While
the driver now loads without issue, no devices appear on the i2c bus:
# i2cdetect -l
i2c-0 smbus SMBus I801 adapter at 5000 SMBus adapter
i2c-1 i2c MCP2221 usb-i2c bridge on hidraw0 I2C adapter
# i2cdetect -y 1
0 1 2 3 4 5 6 7 8 9 a b c d e f
00: -- -- -- -- -- -- -- --
10: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
20: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
30: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
40: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
50: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
60: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
70: -- -- -- -- -- -- -- --
When booting a distribution kernel (Ubunutu 22.04 with kernel 5.15.39) the bus
is populated as expected:
# i2cdetect -l
i2c-0 smbus SMBus CMI adapter cmi SMBus adapter
i2c-1 smbus SMBus iSMT adapter at 20fffab9000 SMBus adapter
i2c-2 smbus SMBus I801 adapter at 5000 SMBus adapter
i2c-3 i2c MCP2221 usb-i2c bridge on hidraw0 I2C adapter
# i2cdetect -y 3
0 1 2 3 4 5 6 7 8 9 a b c d e f
00: -- -- -- -- -- -- -- --
10: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
20: 20 -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
30: -- -- -- -- -- -- -- -- -- -- -- 3b -- -- -- --
40: -- -- -- -- -- -- -- -- UU -- -- -- UU -- -- --
50: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
60: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
70: 70 -- -- -- -- -- -- --
In the patches applied by Ubuntu I couldn't find anything that'd explain this
change in behavior. Regarding their kernel configuration the situation is
similar: Even incorporating almost all of Ubuntu's additions (minus some
signing and integrity stuff) into the config I've been using results in the
NULL pointer dereference in mcp_smbus_xfer. Since this still might be a case of
me missing some config options, I'll post my config in a response to this mail.
There seem to be two recent patch series for this driver:
https://lore.kernel.org/all/20221001005208.8010-1-matt.ranostay@konsulko.com/
https://lore.kernel.org/all/20220926202239.16379-1-Enrik.Berkhan@inka.de/
I tested both, but the behavior stays the same.
[attempted fix]
--- a/drivers/hid/hid-mcp2221.c 2022-09-19 09:31:22.539691089 +0200
+++ b/drivers/hid/hid-mcp2221.c 2022-09-20 15:21:37.576196331 +0200
@@ -873,12 +873,22 @@
"MCP2221 usb-i2c bridge on hidraw%d",
((struct hidraw *)hdev->hidraw)->minor);
+ int adapdata_change = 0;
+ if(!i2c_get_adapdata(&mcp->adapter))
+ {
+ adapdata_change = 1;
+ hid_warn(hdev, "got NULL adapdata\n");
+ i2c_set_adapdata(&mcp->adapter, mcp);
+ }
ret = i2c_add_adapter(&mcp->adapter);
if (ret) {
+ if(adapdata_change)
+ {
+ i2c_set_adapdata(&mcp->adapter, NULL);
+ }
hid_err(hdev, "can't add usb-i2c adapter: %d\n", ret);
goto err_i2c;
}
- i2c_set_adapdata(&mcp->adapter, mcp);
/* Setup GPIO chip */
mcp->gc = devm_kzalloc(&hdev->dev, sizeof(*mcp->gc), GFP_KERNEL);
[dmesg output]
Shortened to just the trace, I'll post the full dmesg output in a response to
this mail.
[ 1.898107] usb 1-3.4: new full-speed USB device number 5 using xhci_hcd
[ 1.990607] cdc_acm 1-3.4:1.0: ttyACM1: USB ACM device
[ 1.991509] hid-generic 0003:04D8:00DD.0003: hiddev96,hidraw2: USB HID v1.11 Device [Microchip Technology Inc. MCP2221 USB-I2C/UART Combo] on usb-0000:00:1e.0-3.4/input2
[ 1.998390] mcp2221 0003:04D8:00DD.0003: hidraw2: USB HID v1.11 Device [Microchip Technology Inc. MCP2221 USB-I2C/UART Combo] on usb-0000:00:1e.0-3.4/input2
[ 2.050149] i2c_dev: adapter [MCP2221 usb-i2c bridge on hidraw2] registered as minor 1
[ 2.050166] i2c i2c-1: adapter [MCP2221 usb-i2c bridge on hidraw2] registered
[ 2.050173] i2c i2c-1: found normal entry for adapter 1, addr 0x48
[ 2.050179] BUG: kernel NULL pointer dereference, address: 0000000000000000
[ 2.057180] #PF: supervisor read access in kernel mode
[ 2.062347] #PF: error_code(0x0000) - not-present page
[ 2.067517] PGD 0 P4D 0
[ 2.070069] Oops: 0000 [#1] PREEMPT SMP
[ 2.073931] CPU: 7 PID: 287 Comm: systemd-udevd Not tainted 6.1.0-rc1-yocto-standard-upstream #1
[ 2.082761] Hardware name: Default string Default string/Default string, BIOS 1.01.10.AR01 08/05/2022
[ 2.092025] RIP: 0010:mcp_smbus_xfer+0x29/0x348 [hid_mcp2221]
[ 2.097811] Code: 00 0f 1f 44 00 00 55 48 89 e5 41 57 41 56 41 55 45 89 cd 41 54 45 89 c4 53 48 83 ec 08 48 8b 9f e8 00 00 00 89 75 d4 89 4d d0 <48> 8b 3b 48 8b 87 b8 1b 00 00 48 8b 40 20 48 85 c0 74 07 be 20 00
[ 2.116673] RSP: 0018:ffffa6c4409ab748 EFLAGS: 00010296
[ 2.121929] RAX: ffffffffc0509fc0 RBX: 0000000000000000 RCX: 0000000000000000
[ 2.129104] RDX: 0000000000000000 RSI: 0000000000000048 RDI: ffff9957c69b5830
[ 2.136278] RBP: ffffa6c4409ab778 R08: 0000000000000000 R09: 0000000000000000
[ 2.143450] R10: ffff995b3fede000 R11: ffff995b3ff8e000 R12: 0000000000000000
[ 2.150625] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
[ 2.157797] FS: 00007f5b4ee65600(0000) GS:ffff995b301c0000(0000) knlGS:0000000000000000
[ 2.165932] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 2.171710] CR2: 0000000000000000 CR3: 0000000108c9b002 CR4: 0000000000770ee0
[ 2.178884] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 2.186059] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 2.193232] PKRU: 55555554
[ 2.195959] Call Trace:
[ 2.198423] <TASK>
[ 2.200540] __i2c_smbus_xfer+0x105/0x3d0
[ 2.204578] ? mcp_i2c_xfer+0x160/0x160 [hid_mcp2221]
[ 2.209663] i2c_smbus_xfer+0x62/0xe0
[ 2.213350] i2c_default_probe+0xf1/0x130
[ 2.217384] i2c_detect.isra.0.cold+0xf4/0x220
[ 2.229989] ? kernfs_add_one+0xe8/0x130
[ 2.233941] ? preempt_count_add+0x7a/0xc0
[ 2.238063] ? _raw_spin_lock_irqsave+0x1e/0x50
[ 2.242623] ? __process_new_driver+0x30/0x30
[ 2.247007] __process_new_adapter+0x18/0x20
[ 2.251304] bus_for_each_drv+0x82/0xd0
[ 2.255167] i2c_register_adapter.cold+0x133/0x1f4
[ 2.259990] i2c_add_adapter+0x5c/0x80
[ 2.263764] mcp2221_probe+0x138/0x250 [hid_mcp2221]
[ 2.268762] ? hid_lookup_quirk+0x7f/0x190
[ 2.272884] hid_device_probe+0xf5/0x160
[ 2.285748] really_probe+0xdf/0x290
[ 2.298964] ? pm_runtime_barrier+0x55/0x90
[ 2.311392] __driver_probe_device+0x78/0xe0
[ 2.323310] driver_probe_device+0x24/0xe0
[ 2.327431] __device_attach_driver+0x7d/0x100
[ 2.337243] ? driver_allows_async_probing+0x60/0x60
[ 2.348108] bus_for_each_drv+0x82/0xd0
[ 2.351969] ? __hid_register_driver+0x90/0x90
[ 2.356439] __device_attach+0xc1/0x1f0
[ 2.360302] ? __hid_register_driver+0x90/0x90
[ 2.364773] device_attach+0x10/0x20
[ 2.368372] device_reprobe+0x4a/0x90
[ 2.372059] __hid_bus_reprobe_drivers+0x56/0x60
[ 2.376704] bus_for_each_dev+0x7c/0xc0
[ 2.386344] ? hid_destroy_device+0x60/0x60
[ 2.390554] __hid_bus_driver_added+0x2c/0x40
[ 2.394937] bus_for_each_drv+0x82/0xd0
[ 2.398800] __hid_register_driver+0x7d/0x90
[ 2.403096] ? 0xffffffffc07c2000
[ 2.406434] mcp2221_driver_init+0x23/0x1000 [hid_mcp2221]
[ 2.411955] do_one_initcall+0x4f/0x210
[ 2.415817] ? kmalloc_trace+0x2a/0xa0
[ 2.419593] do_init_module+0x52/0x200
[ 2.423367] load_module+0x1a78/0x1c50
[ 2.427142] __do_sys_finit_module+0xbb/0x110
[ 2.431525] ? __do_sys_finit_module+0xbb/0x110
[ 2.436086] __x64_sys_finit_module+0x18/0x20
[ 2.440469] do_syscall_64+0x3d/0x90
[ 2.444070] entry_SYSCALL_64_after_hwframe+0x46/0xb0
[ 2.449154] RIP: 0033:0x7f5b4efc202d
[ 2.452755] Code: c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d c3 4d 0e 00 f7 d8 64 89 01 48
[ 2.471614] RSP: 002b:00007ffe16a04a18 EFLAGS: 00000246 ORIG_RAX: 0000000000000139
[ 2.479225] RAX: ffffffffffffffda RBX: 00005651bb4c0670 RCX: 00007f5b4efc202d
[ 2.479226] RDX: 0000000000000000 RSI: 00007f5b4f13636b RDI: 0000000000000006
[ 2.479227] RBP: 00007ffe16a04a40 R08: 0000000000000000 R09: 0000000000000000
[ 2.491744] R10: 0000000000000006 R11: 0000000000000246 R12: 00007f5b4f13636b
[ 2.491745] R13: 0000000000000000 R14: 0000000000020000 R15: 00005651bb4c0670
[ 2.491748] </TASK>
[ 2.506091] Modules linked in: hid_mcp2221(+) ice(+) igc
[ 2.519128] sch_fq_codel tmp421 configfs
[ 2.519132] CR2: 0000000000000000
[ 2.528505] ---[ end trace 0000000000000000 ]---
[ 2.566155] RIP: 0010:mcp_smbus_xfer+0x29/0x348 [hid_mcp2221]
[ 2.587646] Code: 00 0f 1f 44 00 00 55 48 89 e5 41 57 41 56 41 55 45 89 cd 41 54 45 89 c4 53 48 83 ec 08 48 8b 9f e8 00 00 00 89 75 d4 89 4d d0 <48> 8b 3b 48 8b 87 b8 1b 00 00 48 8b 40 20 48 85 c0 74 07 be 20 00
[ 2.587648] RSP: 0018:ffffa6c4409ab748 EFLAGS: 00010296
[ 2.598855] RAX: ffffffffc0509fc0 RBX: 0000000000000000 RCX: 0000000000000000
[ 2.598856] RDX: 0000000000000000 RSI: 0000000000000048 RDI: ffff9957c69b5830
[ 2.608406] RBP: ffffa6c4409ab778 R08: 0000000000000000 R09: 0000000000000000
[ 2.608407] R10: ffff995b3fede000 R11: ffff995b3ff8e000 R12: 0000000000000000
[ 2.608408] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
[ 2.618830] FS: 00007f5b4ee65600(0000) GS:ffff995b301c0000(0000) knlGS:0000000000000000
[ 2.618832] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 2.642332] CR2: 0000000000000000 CR3: 0000000108c9b002 CR4: 0000000000770ee0
[ 2.642333] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 2.642334] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 2.642335] PKRU: 55555554
--
Sven Zühlsdorf
Software Engineer
ViGEM GmbH
next reply other threads:[~2022-10-25 7:40 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-25 7:39 Sven Zühlsdorf [this message]
2022-10-25 7:46 ` NULL pointer dereferences in hid-mcp2221 -- kernel config Sven Zühlsdorf
2022-10-25 7:47 ` NULL pointer dereferences in hid-mcp2221 -- full dmesg Sven Zühlsdorf
2022-11-03 0:31 ` NULL pointer dereferences in hid-mcp2221 Randy Dunlap
2022-11-03 12:16 ` Benjamin Tissoires
2022-11-03 15:22 ` Enrik Berkhan
2022-11-03 16:13 ` Randy Dunlap
2022-12-13 14:00 ` Sven Zühlsdorf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=79152feb-bcbc-9e3e-e776-13170ae4ef40@vigem.de \
--to=sven.zuehlsdorf@vigem.de \
--cc=gupt21@gmail.com \
--cc=linux-i2c@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.