Re: [PATCH] wifi: brcm80211: handle pmk_op allocation failure

From: Arend van Spriel
Date: Tue Feb 27 2024 - 06:22:10 EST

On 2/27/2024 9:06 AM, Duoming Zhou wrote:
The kzalloc() in brcmf_pmksa_v3_op() will return null if the
physical memory has run out. As a result, if we dereference
the null value, the null pointer dereference bug will happen.

Return -ENOMEM from brcmf_pmksa_v3_op() if kzalloc() fails
for pmk_op.

NAK (see below)

Also this issue was reported earlier by Joe Perches. Not sure if he wants to be mentioned as such.

Fixes: a96202acaea4 ("wifi: brcmfmac: cfg80211: Add support for PMKID_V3 operations")
Signed-off-by: Duoming Zhou <duoming@xxxxxxxxxx>
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 28d6a30cc01..3b420b33188 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -4322,6 +4322,10 @@ brcmf_pmksa_v3_op(struct brcmf_if *ifp, struct cfg80211_pmksa *pmksa,
int ret;
pmk_op = kzalloc(sizeof(*pmk_op), GFP_KERNEL);
+ if (!pmk_op) {
+ ret = -ENOMEM;
+ goto out;
+ }

There is really no need to introduce a new label for this. Although you can kfree() a NULL pointer there is no need to do so when you know already it is NULL. Just return -ENOMEM and be done with it.

Regards,
Arend

pmk_op->version = cpu_to_le16(BRCMF_PMKSA_VER_3);
if (!pmksa) {
@@ -4340,6 +4344,7 @@ brcmf_pmksa_v3_op(struct brcmf_if *ifp, struct cfg80211_pmksa *pmksa,
pmk_op->length = cpu_to_le16(length);
ret = brcmf_fil_iovar_data_set(ifp, "pmkid_info", pmk_op, sizeof(*pmk_op));
+out:
kfree(pmk_op);
return ret;
}

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature