On Tue, Jul 18, 2023 at 02:07:00PM +0200, Milan Zamazal wrote:
> Commit 813665564b3d ("iio: core: Convert to use firmware node handle
> instead of OF node") switched the kind of nodes to use for label
> retrieval in device registration. Probably an unwanted change in that
> commit was that if the device has no parent then NULL pointer is
> accessed. This is what happens in the stock IIO dummy driver when a
> new entry is created in configfs:
> # mkdir /sys/kernel/config/iio/devices/dummy/foo
> BUG: kernel NULL pointer dereference, address: 0000000000000278
> ...
> ? asm_exc_page_fault+0x22/0x30
> ? container_offline+0x20/0x20
> __iio_device_register+0x45/0xc10
> ? krealloc+0x73/0xa0
> ? iio_device_attach_buffer+0x31/0xc0
> ? iio_simple_dummy_configure_buffer+0x20/0x20
> ? iio_triggered_buffer_setup_ext+0xb4/0x100
> iio_dummy_probe+0x112/0x190
> iio_sw_device_create+0xa8/0xd0
> device_make_group+0xe/0x40
> configfs_mkdir+0x1a6/0x440
Read this
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#backtraces-in-commit-messages
and amend the commit message accordingly.
> Since there seems to be no reason to make a parent device of an IIO
> dummy device mandatory, let’s prevent the invalid memory access in
> __iio_device_register when the parent device is NULL. With this
> change, the IIO dummy driver works fine with configfs.
...
> int __iio_device_register(struct iio_dev *indio_dev, struct module *this_mod)
> {
> struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev);
> - struct fwnode_handle *fwnode;
> + struct fwnode_handle *fwnode = NULL;
> int ret;
>
> if (!indio_dev->info)
> /* If the calling driver did not initialize firmware node, do it here */
> if (dev_fwnode(&indio_dev->dev))
> fwnode = dev_fwnode(&indio_dev->dev);
> - else
> + else if (indio_dev->dev.parent != NULL)
> fwnode = dev_fwnode(indio_dev->dev.parent);
This part of the change is okay with dropped " != NULL".
> - device_set_node(&indio_dev->dev, fwnode);
> -
> - fwnode_property_read_string(fwnode, "label", &indio_dev->label);
> + if (fwnode != NULL) {
> + device_set_node(&indio_dev->dev, fwnode);
> + fwnode_property_read_string(fwnode, "label", &indio_dev->label);
> + }
This part of the change is not needed.
--
With Best Regards,
Andy Shevchenko
