On Tue, 25 Oct 2022, Sven Zühlsdorf wrote:
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:
[...]
When booting a distribution kernel (Ubunutu 22.04 with kernel 5.15.39) the
bus
is populated as expected:
[...]
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@xxxxxxxxxxxx/
https://lore.kernel.org/all/20220926202239.16379-1-Enrik.Berkhan@xxxxxxx/
I tested both, but the behavior stays the same.
[...]
We managed to completely fix our second issue. While my initial patch did
fix the observed NULL pointer dereference, the second patch from Enrik
Berkhan's series could have clued me in to what was missing: The hid device
needs to be started, otherwise the responses from the chip aren't processed.
Patch, based on hid.git for-6.2/mcp2221 3d74c9eca1a2, in reply.
The differences to the kernel shipped by Ubuntu turned out to be a red
herring. What actually triggered the NULL dereference was having a driver
loaded that actively probes new I2C busses for suitable devices, thus
triggering the mcp_*_xfer functions before the adapdata was set. Unloading
and reloading hid_mcp2221 once a suitable driver is loaded will happily
trigger the bug on any kernel.
