On Fri, 19 Feb 2021 10:58:25 +0200
Alexandru Ardelean <alexandru.ardelean@xxxxxxxxxx> wrote:
> When the buffer attributes were wrapped in iio_dev_attr types, I forgot to
> duplicate the names, so that when iio_free_chan_devattr_list() gets called
> on cleanup, these get free'd.
> I stumbled over this while accidentally breaking a driver doing
> iio_device_register(), and then the issue appeared.
>
> The fix can be just
> 1. Just use kstrdup() during iio_buffer_wrap_attr()
> 2. Just use kfree_const() during iio_free_chan_devattr_list
> 3. Use both kstrdup_const() & kfree_const() (in the places mentioned above)
>
> Using kfree_const() should be sufficient, as the attribute names will
> either be allocated or be stored in rodata.
>
> Fixes: a1a11142f66c ("iio: buffer: wrap all buffer attributes into iio_dev_attr")
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@xxxxxxxxxx>
Thinking more on this... It's fine for the users today, but there is
nothing stopping a driver passing in names it allocated on the heap. So
I think we should revisit this. Perhaps we need 1 or 3.
> ---
> drivers/iio/industrialio-core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index 0d8c6e88d993..cb2735d2ae4b 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -1358,7 +1358,7 @@ void iio_free_chan_devattr_list(struct list_head *attr_list)
> struct iio_dev_attr *p, *n;
>
> list_for_each_entry_safe(p, n, attr_list, l) {
> - kfree(p->dev_attr.attr.name);
> + kfree_const(p->dev_attr.attr.name);
> list_del(&p->l);
> kfree(p);
> }
