Received-SPF: pass (google.com: domain of linux-kernel+bounces-85209-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Wed, 28 Feb 2024 06:48:14 -0800
From: Kees Cook <kees@kernel.org>
To: Duoming Zhou <duoming@zju.edu.cn>, linux-kernel@vger.kernel.org
CC: brcm80211-dev-list.pdl@broadcom.com, brcm80211@lists.linux.dev,
 linux-wireless@vger.kernel.org, justinstitt@google.com,
 jisoo.jang@yonsei.ac.kr, petr.tesarik.ext@huawei.com,
 quic_alokad@quicinc.com, hdegoede@redhat.com, keescook@chromium.org,
 johannes.berg@intel.com, kvalo@kernel.org, arend.vanspriel@broadcom.com
Subject: Re: [PATCH v2] wifi: brcm80211: handle pmk_op allocation failure
User-Agent: K-9 Mail for Android
In-Reply-To: <20240228063408.7006-1-duoming@zju.edu.cn>
References: <20240228063408.7006-1-duoming@zju.edu.cn>
Message-ID: <8B696743-9487-4A78-B9B4-16DA4F2F8E26@kernel.org>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain;
 charset=utf-8
Content-Transfer-Encoding: quoted-printable


On February 27, 2024 10:34:08 PM PST, Duoming Zhou <duoming@zju=2Eedu=2Ecn=
> wrote:
>The kzalloc() in brcmf_pmksa_v3_op() will return null if the
>physical memory has run out=2E As a result, if we dereference
>the null value, the null pointer dereference bug will happen=2E
>
>Return -ENOMEM from brcmf_pmksa_v3_op() if kzalloc() fails
>for pmk_op=2E
>
>Fixes: a96202acaea4 ("wifi: brcmfmac: cfg80211: Add support for PMKID_V3 =
operations")
>Signed-off-by: Duoming Zhou <duoming@zju=2Eedu=2Ecn>
>---
>Changes in v2:
>  - Drop the new label and just return -ENOMEM=2E
>
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211=2Ec | 3 +++
> 1 file changed, 3 insertions(+)
>
>diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211=2E=
c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211=2Ec
>index 28d6a30cc01=2E=2E7af6d6448b9 100644
>--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211=2Ec
>+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211=2Ec
>@@ -4322,6 +4322,9 @@ brcmf_pmksa_v3_op(struct brcmf_if *ifp, struct cfg8=
0211_pmksa *pmksa,
> 	int ret;
>=20
> 	pmk_op =3D kzalloc(sizeof(*pmk_op), GFP_KERNEL);
>+	if (!pmk_op)
>+		ret =3D -ENOMEM;

This doesn't fix anything=2E It doesn't stop the execution path; it'll con=
tinue and immediately dereference the NULL pmk_op in the next line=2E=2E=2E

>+
> 	pmk_op->version =3D cpu_to_le16(BRCMF_PMKSA_VER_3);
>=20
> 	if (!pmksa) {

--=20
Kees Cook