lkml.org 
[lkml]   [2012]   [Jul]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: NULL pointer dereference in iio_buffer_register()
    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


    \
     
     \ /
      Last update: 2012-07-31 15:41    [W:0.036 / U:60.340 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site