Re: NULL pointer dereference in iio_buffer_register()

Lars-Peter Clausen
2012-07-31T13:01:59Z

On 07/31/2012 02:55 PM, Fengguang Wu wrote:
>>> The panic happens while trying to dereference the NULL indio_dev->buffer:
>>>
>>> 266 int iio_buffer_register(struct iio_dev *indio_dev,
>>> 267 const struct iio_chan_spec *channels,
>>> 268 int num_channels)
>>> 269 {
>>> 270 struct iio_dev_attr *p;
>>> 271 struct attribute **attr;
>>> 272 struct iio_buffer *buffer = indio_dev->buffer;
>>> 273 int ret, i, attrn, attrcount, attrcount_orig = 0;
>>> 274
>>> ==> 275 if (buffer->attrs)
>>> 276 indio_dev->groups[indio_dev->groupcounter++] = buffer->attrs;
>>>
>>> iio_dummy_probe() has the code to configure that buffer, however
>>> iio_simple_dummy_configure_buffer() is defined to do nothing on
>>> !CONFIG_IIO_SIMPLE_DUMMY_BUFFER..
>>>
>>> 448 /* Configure buffered capture support. */
>>> ==> 449 ret = iio_simple_dummy_configure_buffer(indio_dev);
>>> 450 if (ret < 0)
>>> 451 goto error_unregister_events;
>>> 452
>>> 453 /*
>>> 454 * Register the channels with the buffer, but avoid the output
>>> 455 * channel being registered by reducing the number of channels by 1.
>>> 456 */
>>> 457 ret = iio_buffer_register(indio_dev, iio_dummy_channels, 5);
>>> 458 if (ret < 0)
>>> 459 goto error_unconfigure_buffer;
>>>
>>> Any ideas to fix it?
>>>
>>
>> Hi,
>>
>> I think the best would be to move the iio_buffer_register to
>> iio_simple_dummy_configure_buffer.
>
> Lars, thanks for the quick reply! Hmm, that looks more like a code
> refactor recommendation than fix ;) In the simplest form, can the
> bug fixed like this?
>
> static inline int iio_simple_dummy_configure_buffer(struct iio_dev *indio_dev)
> {
> - return 0;
> + return -1;
> };
>

No, we want iio_simple_dummy_configure_buffer to be a noop if buffer support
is disabled, since the driver works fine without it. Except for the issue
you discovered. This issue only appears if CONFIG_IIO_BUFFER=y and
CONFIG_IIO_SIMPLE_DUMMY_BUFFER=n. E.g. if both are not set the driver works
fine without buffers.

I can prepare a patch which moves the iio_buffer_register to
iio_simple_dummy_configure_buffer if you want to.

- Lars