Skip to contentSkip to conversation

staging: vchiq: silence an uninitialized variable warning

PATCH Superseded Dan Carpenter 2 participants 4 messages Sep 30, 2020

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 staging: vchiq: silence an uninitialized variable warning

Smatch complains that "userdata" can be passed to vchiq_bulk_transfer() without being initialized. Smatch is correct, however, in that situation the "userdata" is not used so it doesn't cause a problem. Passing an uninitialized variable will trigger a UBSan warning at runtime so this warning is worth silencing by setting "userdata" to NULL.

Fixes a4367cd2b231 ("staging: vchiq: convert compat bulk transfer")
---
 drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c +1 -1
index 590415561b73..71b962777da5 100644
@@ -952,7 +952,7 @@ static int vchiq_irq_queue_bulk_tx_rx(struct vchiq_instance *instance,
 {
 	struct vchiq_service *service;
 	struct bulk_waiter_node *waiter = NULL;
-	void *userdata;
+	void *userdata = NULL;
 	int status = 0;
 	int ret;
 
Signature · 1 line
2.28.0
Acked-by on v1 1/1
#1 lore raw
Quoted 10 lines · Dan Carpenter

Smatch complains that "userdata" can be passed to vchiq_bulk_transfer() without being initialized. Smatch is correct, however, in that situation the "userdata" is not used so it doesn't cause a problem. Passing an uninitialized variable will trigger a UBSan warning at runtime so this warning is worth silencing by setting "userdata" to NULL.

Fixes a4367cd2b231 ("staging: vchiq: convert compat bulk transfer")

The change looks fine, but I wonder if it's actually worse and the uninitialized pointer can end up getting copied back to user space in the completion.

In either case, thanks for the fix!

#2 lore raw
Quoted 9 lines · Arnd Bergmann
Quoted 8 lines · Dan Carpenter

Smatch complains that "userdata" can be passed to vchiq_bulk_transfer() without being initialized. Smatch is correct, however, in that situation the "userdata" is not used so it doesn't cause a problem. Passing an uninitialized variable will trigger a UBSan warning at runtime so this warning is worth silencing by setting "userdata" to NULL.

Quoted 6 lines
Quoted 2 lines
Fixes a4367cd2b231 ("staging: vchiq: convert compat bulk transfer")

The change looks fine, but I wonder if it's actually worse and the uninitialized pointer can end up getting copied back to user space in the completion.

Ah... Wow. You're right. I think I really need to resend this with a more accurate commit message.

regards,
dan carpenter

#3 lore raw

PATCH v2 1/1 staging: vchiq: Fix an uninitialized variable

In reply to Arnd Bergmann

Smatch complains that "userdata" can be passed to vchiq_bulk_transfer() without being initialized. This leads to a potential information leak later on.

Fixes a4367cd2b231 ("staging: vchiq: convert compat bulk transfer")

---
v2: update commit message.

 drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c +1 -1
index 590415561b73..71b962777da5 100644
@@ -952,7 +952,7 @@ static int vchiq_irq_queue_bulk_tx_rx(struct vchiq_instance *instance,
 {
 	struct vchiq_service *service;
 	struct bulk_waiter_node *waiter = NULL;
-	void *userdata;
+	void *userdata = NULL;
 	int status = 0;
 	int ret;
 
Signature · 6 lines
2.28.0

_______________________________________________
devel mailing list
[email protected]
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel