Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Sun, 6 Nov 2022 17:27:24 +0100
From:   Uwe =?utf-8?Q?Kleine-K=C3=B6nig?= <u.kleine-koenig@pengutronix.de>
To:     Yang Yingliang <yangyingliang@huawei.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc:     linux-kernel@vger.kernel.org,
        Thorsten Scherer <t.scherer@eckelmann.de>,
        kernel@pengutronix.de
Subject: Re: [PATCH] siox: fix possible memory leak in siox_device_add()
Message-ID: <20221106162724.dwlwep32wxkcxd5z@pengutronix.de>
References: <20221104021334.618189-1-yangyingliang@huawei.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="27dzadorfidvq2uu"
Content-Disposition: inline
In-Reply-To: <20221104021334.618189-1-yangyingliang@huawei.com>
Precedence: bulk


--27dzadorfidvq2uu
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Hello,

On Fri, Nov 04, 2022 at 10:13:34AM +0800, Yang Yingliang wrote:
> If device_register() returns error in siox_device_add(),
> the name allocated by dev_set_name() need be freed. As
> comment of device_register() says, it should use put_device()
> to give up the reference in the error path. So fix this
> by calling put_device(), then the name can be freed in
> kobject_cleanup(), and sdevice is freed in siox_device_release(),
> set it to null in error path.
>=20
> Fixes: bbecb07fa0af ("siox: new driver framework for eckelmann SIOX")
> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
> ---
>  drivers/siox/siox-core.c | 2 ++
>  1 file changed, 2 insertions(+)
>=20
> diff --git a/drivers/siox/siox-core.c b/drivers/siox/siox-core.c
> index 7c4f32d76966..561408583b2b 100644
> --- a/drivers/siox/siox-core.c
> +++ b/drivers/siox/siox-core.c
> @@ -839,6 +839,8 @@ static struct siox_device *siox_device_add(struct sio=
x_master *smaster,
> =20
>  err_device_register:
>  	/* don't care to make the buffer smaller again */
> +	put_device(&sdevice->dev);
> +	sdevice =3D NULL;

The problem is real and the fix works as intended. I don't like the
necessity to set sdevice to NULL, but that's the easiest approach here,
so:

Reviewed-by: Uwe Kleine-K=F6nig <u.kleine-koenig@pengutronix.de>

>  err_buf_alloc:
>  	siox_master_unlock(smaster);

@gregkh: Could you pick up this patch please[1]? While I don't consider it
very urgent, it's technically a fix. I don't care much if it goes in now
or during the next merge window.

Best regards
Uwe

[1] the original Message-Id is
    20221104021334.618189-1-yangyingliang@huawei.com. Maybe this helps
    you to apply the patch even if you didn't get a Cc.

--=20
Pengutronix e.K.                           | Uwe Kleine-K=F6nig            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

--27dzadorfidvq2uu
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQEzBAABCgAdFiEEfnIqFpAYrP8+dKQLwfwUeK3K7AkFAmNn4FgACgkQwfwUeK3K
7Ak/GQf+Pw+TSsEGpowlbtHa+ekY78jy1eQAcWiQr9jxJGznBF4Ym13NKvszWzug
lnLCLG44KDB19BaTnzIYB+vXCJJas7dUpR/gy0Yl2XtjQgpEHEhp+wCY1bfpaQMY
64+dGyaFmYaY0sz5kphJgGRxBjgn6E34e06Tphv/qmwv46aApFzCV1FGpp7pDbXJ
bmJXY+uJCfEcKBKqIAwWgDxyMxY6RzfAExejZJs7Z8FwFm+MMDrvxQNd9pP1hnAB
Qr0aevg7B/xbEbniqr8fLnpjINc8YsP5W3W9tgEXYPAstoplG8talpgElDmtnLpU
szRIWs6pQ3svPILvZKQV6yadczkJSg==
=nzFJ
-----END PGP SIGNATURE-----

--27dzadorfidvq2uu--