linux-kernel - [PATCH RESEND] drivers:media:radio: Fix atomicity violation in fmc_send

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 linux-cve-announce PHC
Open Source and information security mailing list archives

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

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

Message-Id: <20241030064824.6122-1-chenqiuji666@gmail.com>
Date: Wed, 30 Oct 2024 14:48:24 +0800
From: Qiu-ji Chen <chenqiuji666@...il.com>
To: mchehab@...nel.org,
	hverkuil-cisco@...all.nl,
	allen.lkml@...il.com
Cc: linux-media@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	baijiaju1990@...il.com,
	Qiu-ji Chen <chenqiuji666@...il.com>,
	stable@...r.kernel.org
Subject: [PATCH RESEND] drivers:media:radio: Fix atomicity violation in fmc_send_cmd()

Atomicity violation occurs when the fmc_send_cmd() function is executed 
simultaneously with the modification of the fmdev->resp_skb value. 
Consider a scenario where, after passing the validity check within the 
function, a non-null fmdev->resp_skb variable is assigned a null value. 
This results in an invalid fmdev->resp_skb variable passing the validity 
check. As seen in the later part of the function, skb = fmdev->resp_skb; 
when the invalid fmdev->resp_skb passes the check, a null pointer 
dereference error may occur at line 478, evt_hdr = (void *)skb->data;

To address this issue, it is recommended to include the validity check of 
fmdev->resp_skb within the locked section of the function. This 
modification ensures that the value of fmdev->resp_skb does not change 
during the validation process, thereby maintaining its validity.

This possible bug is found by an experimental static analysis tool
developed by our team. This tool analyzes the locking APIs
to extract function pairs that can be concurrently executed, and then
analyzes the instructions in the paired functions to identify possible
concurrency bugs including data races and atomicity violations.

Fixes: e8454ff7b9a4 ("[media] drivers:media:radio: wl128x: FM Driver Common sources")
Cc: stable@...r.kernel.org
Signed-off-by: Qiu-ji Chen <chenqiuji666@...il.com>
---
 drivers/media/radio/wl128x/fmdrv_common.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/radio/wl128x/fmdrv_common.c b/drivers/media/radio/wl128x/fmdrv_common.c
index 3d36f323a8f8..4d032436691c 100644
--- a/drivers/media/radio/wl128x/fmdrv_common.c
+++ b/drivers/media/radio/wl128x/fmdrv_common.c
@@ -466,11 +466,12 @@ int fmc_send_cmd(struct fmdev *fmdev, u8 fm_op, u16 type, void *payload,
 			   jiffies_to_msecs(FM_DRV_TX_TIMEOUT) / 1000);
 		return -ETIMEDOUT;
 	}
+	spin_lock_irqsave(&fmdev->resp_skb_lock, flags);
 	if (!fmdev->resp_skb) {
+		spin_unlock_irqrestore(&fmdev->resp_skb_lock, flags);
 		fmerr("Response SKB is missing\n");
 		return -EFAULT;
 	}
-	spin_lock_irqsave(&fmdev->resp_skb_lock, flags);
 	skb = fmdev->resp_skb;
 	fmdev->resp_skb = NULL;
 	spin_unlock_irqrestore(&fmdev->resp_skb_lock, flags);
-- 
2.34.1