Skip to contentSkip to conversation

staging: tidspbridge: Fix potential NULL pointer dereference

PATCH In review Emil Goode 2 participants 3 messages Jun 12, 2013

Subsystems (2)
lore
Share & tools
lore

Emil Goode

author
#0 lore raw

PATCH 1/1 staging: tidspbridge: Fix potential NULL pointer dereference

We free dcd_key on line 897 and then dereference it a few lines below. This patch adds a NULL check to make sure we can use dcd_key.

---
 drivers/staging/tidspbridge/rmgr/dbdcd.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/staging/tidspbridge/rmgr/dbdcd.c +1 -1
index 3d2a26f..21ce3b7 100644
@@ -899,7 +899,7 @@ int dcd_register_object(struct dsp_uuid *uuid_obj,
 			}
 		}
 		spin_unlock(&dbdcd_lock);
-		if (&dcd_key->link = &reg_key_list)
+		if (dcd_key && (&dcd_key->link = &reg_key_list))
 			status = -EPERM;
 	}
 
Signature · 1 line
1.7.10.4
#1 lore raw

1) This patch doesn't do anything. "dcd_key" points to freed memory but it's not a NULL pointer.
2) The original pointer doesn't dereference "dcd_key" it just takes the address of one of the elements. I don't know the context and can't say for sure that it's a wrong thing to do.

The code does look very suspect. Why is it checking permisions after we already removed the element and freed it?

But it's not a dereferencing freed memory bug.

regards,
dan carpenter

author
#2 lore raw

Thank you Dan! That's right, It's of course not NULL after freed. Yes the code looks strange here, I'm not sure what was intended.

Best regards,

Emil

Quoted 14 lines · Dan Carpenter

1) This patch doesn't do anything. "dcd_key" points to freed memory but it's not a NULL pointer.
2) The original pointer doesn't dereference "dcd_key" it just takes the address of one of the elements. I don't know the context and can't say for sure that it's a wrong thing to do.

The code does look very suspect. Why is it checking permisions after we already removed the element and freed it?

But it's not a dereferencing freed memory bug.

regards,
dan carpenter