linux-kernel - Re: [PATCH] iio: buffer: Fix memory leak in iio_buffer_register_legacy_sysfs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Sun, 17 Oct 2021 15:43:54 +0100
From:   Jonathan Cameron <jic23@...nel.org>
To:     Yang Yingliang <yangyingliang@...wei.com>
Cc:     <linux-kernel@...r.kernel.org>, <linux-iio@...r.kernel.org>,
        <lars@...afoo.de>, <ardeleanalex@...il.com>
Subject: Re: [PATCH] iio: buffer: Fix memory leak in
 iio_buffer_register_legacy_sysfs_groups()

On Wed, 13 Oct 2021 22:42:42 +0800
Yang Yingliang <yangyingliang@...wei.com> wrote:

> If the second iio_device_register_sysfs_group() fails,
> 'legacy_buffer_group.attrs' need be freed too or it will
> cause memory leak:
> 
> unreferenced object 0xffff888003618280 (size 64):
>   comm "xrun", pid 357, jiffies 4294907259 (age 22.296s)
>   hex dump (first 32 bytes):
>     80 f6 8c 03 80 88 ff ff 80 fb 8c 03 80 88 ff ff  ................
>     00 f9 8c 03 80 88 ff ff 80 fc 8c 03 80 88 ff ff  ................
>   backtrace:
>     [<00000000076bfd43>] __kmalloc+0x1a3/0x2f0
>     [<00000000c32e4886>] iio_buffers_alloc_sysfs_and_mask+0xc31/0x1290 [industrialio]
>     [<000000002fcd0bb8>] __iio_device_register+0x52e/0x1b40 [industrialio]
>     [<000000008116530c>] __devm_iio_device_register+0x22/0x80 [industrialio]
>     [<000000008a47327c>] adjd_s311_probe+0x195/0x200 [adjd_s311]
>     [<00000000f8eeb456>] i2c_device_probe+0xa07/0xbb0
>     [<000000000e86686c>] really_probe+0x285/0xc30
>     [<00000000a49db55c>] __driver_probe_device+0x35f/0x4f0
>     [<00000000d1fd43a1>] driver_probe_device+0x4f/0x140
>     [<000000008cdafdfa>] __device_attach_driver+0x24c/0x330
>     [<000000006466e92e>] bus_for_each_drv+0x15d/0x1e0
>     [<00000000154fbb1c>] __device_attach+0x267/0x410
>     [<000000007c84d5d1>] bus_probe_device+0x1ec/0x2a0
>     [<0000000019967467>] device_add+0xc3d/0x2020
>     [<00000000a16d2d51>] i2c_new_client_device+0x614/0xb00
>     [<00000000a56a901d>] new_device_store+0x1f4/0x410
I cut this down a bit.
> 
> Reported-by: Hulk Robot <hulkci@...wei.com>
> Fixes: d9a625744ed0 ("iio: core: merge buffer/ & scan_elements/ attributes")
> Signed-off-by: Yang Yingliang <yangyingliang@...wei.com>

Ouch on this one.  I guess a classic bit of too much churn and resulting bit rot
leading to this oddity.  At somepoint it would be good to clean this up further
and tidy up the remove order to match this error path.

Applied to the fixes-togreg branch of iio.git and marked for stable.

Thanks,

Jonathan

> ---
>  drivers/iio/industrialio-buffer.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
> index ba1c5c898e53..88741385764e 100644
> --- a/drivers/iio/industrialio-buffer.c
> +++ b/drivers/iio/industrialio-buffer.c
> @@ -1367,10 +1367,10 @@ static int iio_buffer_register_legacy_sysfs_groups(struct iio_dev *indio_dev,
>  
>  	return 0;
>  
> -error_free_buffer_attrs:
> -	kfree(iio_dev_opaque->legacy_buffer_group.attrs);
>  error_free_scan_el_attrs:
>  	kfree(iio_dev_opaque->legacy_scan_el_group.attrs);
> +error_free_buffer_attrs:
> +	kfree(iio_dev_opaque->legacy_buffer_group.attrs);
>  
>  	return ret;
>  }