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;
From:   =?UTF-8?Q?Micha=C5=82_Lach?= <michal.lach@samsung.com>
To:     <linux-kernel@vger.kernel.org>
Cc:     <mcgrof@kernel.org>, <russell.h.weight@intel.com>,
        <gregkh@linuxfoundation.org>, <rafael@kernel.org>,
        <ming.lei@canonical.com>, <tiwai@suse.de>
In-Reply-To: <20221123111455.94972-1-michal.lach@samsung.com>
Subject: RE: [PATCH] drivers/firmware_loader: remove list entry before
 deallocation
Date:   Thu, 8 Dec 2022 11:45:28 +0100
Message-ID: <000901d90af2$309b7c80$91d27580$@samsung.com>
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Thread-Index: AQJJ6E+zfdaFORO17aRh9qp0ZMDCaQIgon1nrXEK0JA=
Content-Language: pl
Content-Type: text/plain; charset="utf-8"
CMS-TYPE: 201P
References: <CGME20221123111806eucas1p23fdcdbe6e5f4a9e714db428fcd6552b9@eucas1p2.samsung.com>
        <20221123111455.94972-1-michal.lach@samsung.com>
Precedence: bulk

Pinging

With regards,
Micha=C5=82=0D=0A=0D=0A>=20-----Original=20Message-----=0D=0A>=20From:=20Mi=
chal=20Lach=20<michal.lach=40samsung.com>=0D=0A>=20Sent:=20=C5=9Broda,=2023=
=20listopada=202022=2012:15=0D=0A>=20To:=20linux-kernel=40vger.kernel.org=
=0D=0A>=20Cc:=20mcgrof=40kernel.org;=20russell.h.weight=40intel.com;=0D=0A>=
=20gregkh=40linuxfoundation.org;=20rafael=40kernel.org;=20ming.lei=40canoni=
cal.com;=0D=0A>=20tiwai=40suse.de;=20michal.lach=40samsung.com=0D=0A>=20Sub=
ject:=20=5BPATCH=5D=20drivers/firmware_loader:=20remove=20list=20entry=20be=
fore=20deallocation=0D=0A>=20=0D=0A>=20If=20CONFIG_FW_LOADER_USER_HELPER=20=
is=20enabled,=20it=20is=20possible=20to=20interrupt=20the=0D=0A>=20loading=
=20process=20after=20adding=20pending_list=20to=20pending_fw_list.=0D=0A>=
=20Subsequently,=20if=20user=20calls=20release_firmware()=20which=20dealloc=
ates=20the=20fw_priv=0D=0A>=20structure=20which=20pending_list=20is=20a=20m=
ember=20of,=20the=20entry=20in=20the=20list=20is=20not=20deleted.=0D=0A>=20=
This=20causes=20a=20use-after-free=20on=20further=20attempts=20to=20add=20a=
n=20entry=20to=20the=20list=20or=20on=0D=0A>=20list=20traversal.=0D=0A>=20=
=0D=0A>=20While=20not=20problematic=20in=20most=20drivers=20since=20this=20=
function=20is=20mainly=20used=20in=20probe=0D=0A>=20or=20init=20routines,=
=20some=20drivers=20expose=20firmware=20loading=20functionality=20via=20use=
r-=0D=0A>=20accessible=20functions=20like=20write()=20etc.=0D=0A>=20In=20th=
is=20case=20during=20the=20sysfs=20loading=20process,=20we=20can=20send=20S=
IGKILL=20to=20the=20thread=0D=0A>=20which=20is=20then=20in=20kernel,=20leav=
e=20the=20entry=20in=20the=20list=20and=20then=20free=20the=20structure.=0D=
=0A>=20=0D=0A>=20Example=20kernel=20panics=20with=20CONFIG_DEBUG_LIST=20tur=
ned=20on:=0D=0A>=20=0D=0A>=20kernel=20BUG=20at=20lib/list_debug.c:25=21=0D=
=0A>=20/*=20...=20*/=0D=0A>=20Call=20trace:=0D=0A>=20=20__list_add_valid+0x=
7c/0x98=0D=0A>=20=20fw_load_sysfs_fallback+0xd4/0x334=0D=0A>=20=20fw_load_f=
rom_user_helper+0x148/0x1f8=0D=0A>=20=20firmware_fallback_sysfs+0xe0/0x17c=
=0D=0A>=20=20_request_firmware+0x1a0/0x470=0D=0A>=20=20request_firmware+0x5=
0/0x78=0D=0A>=20/*=20...=20*/=0D=0A>=20=0D=0A>=20or=0D=0A>=20=0D=0A>=20kern=
el=20BUG=20at=20lib/list_debug.c:56=21=0D=0A>=20/*=20...=20*/=0D=0A>=20Call=
=20trace:=0D=0A>=20=20__list_del_entry_valid+0xa0/0xa4=0D=0A>=20=20fw_load_=
abort+0x38/0x64=0D=0A>=20=20fw_load_sysfs_fallback+0x354/0x468=0D=0A>=20=20=
fw_load_from_user_helper+0x17c/0x1c0=0D=0A>=20=20firmware_fallback_sysfs+0x=
c0/0x11c=0D=0A>=20=20_request_firmware+0xe0/0x4a4=0D=0A>=20=20request_firmw=
are+0x20/0x2c=0D=0A>=20/*=20...=20*/=0D=0A>=20=0D=0A>=20Fixes:=20fe304143b0=
c3=20(=22firmware:=20Avoid=20deadlock=20of=20usermodehelper=20lock=20at=0D=
=0A>=20shutdown=22)=0D=0A>=20Signed-off-by:=20Michal=20Lach=20<michal.lach=
=40samsung.com>=0D=0A>=20---=0D=0A>=20=20drivers/base/firmware_loader/main.=
c=20=7C=204=20++++=0D=0A>=20=201=20file=20changed,=204=20insertions(+)=0D=
=0A>=20=0D=0A>=20diff=20--git=20a/drivers/base/firmware_loader/main.c=0D=0A=
>=20b/drivers/base/firmware_loader/main.c=0D=0A>=20index=207c3590fd97c2..38=
1997c84e4f=20100644=0D=0A>=20---=20a/drivers/base/firmware_loader/main.c=0D=
=0A>=20+++=20b/drivers/base/firmware_loader/main.c=0D=0A>=20=40=40=20-209,6=
=20+209,10=20=40=40=20static=20void=20__free_fw_priv(struct=20kref=20*ref)=
=0D=0A>=20=20=09=09=20__func__,=20fw_priv->fw_name,=20fw_priv,=20fw_priv->d=
ata,=0D=0A>=20=20=09=09=20(unsigned=20int)fw_priv->size);=0D=0A>=20=0D=0A>=
=20+=23ifdef=20CONFIG_FW_LOADER_USER_HELPER=0D=0A>=20+=09list_del(&fw_priv-=
>pending_list);=0D=0A>=20+=23endif=0D=0A>=20+=0D=0A>=20=20=09list_del(&fw_p=
riv->list);=0D=0A>=20=20=09spin_unlock(&fwc->lock);=0D=0A>=20=0D=0A>=20--=
=0D=0A>=202.25.1=0D=0A=0D=0A=0D=0A