Bug#256954: frees wrong memory

[barbier@linuxfr.org (Denis Barbier)]

On Tue, Jun 29, 2004 at 09:38:13PM -0700, Matt Kraai wrote:
[...]
> I think the read fails because the other end of the pipe closes
> because cdebconf crashes because of a bug in
> question_owner_delete: if the owner that it is trying to delete is
> not last one in the list, it frees the owner field.
> 
> The attached, untested patch should fix it, but I'd appreciate a
> review.

It seems that the question_owner_delete routine is intended to remove
an entry from a single chained list.  AFAICT the current implementation
as well as your patch do not preserve links when the removed item is
not in list tail.  Here is another patch, not tested.

Denis

Index: packages/cdebconf/src/question.c
===================================================================
--- packages/cdebconf/src/question.c	(revision 17355)
+++ packages/cdebconf/src/question.c	(working copy)
@@ -122,25 +122,22 @@
 
 void question_owner_delete(struct question *q, const char *owner)
 {
-	struct questionowner **ownerp, *nextp;
+	struct questionowner *ownerp, *nextp, *prevp = NULL;
 
-	for (ownerp = &q->owners; *ownerp != 0; ownerp = &(*ownerp)->next)
+	for (ownerp = q->owners; ownerp != NULL; ownerp = nextp)
 	{
-		if (strcmp((*ownerp)->owner, owner) == 0)
+		nextp = ownerp->next;
+		if (strcmp(ownerp->owner, owner) == 0)
 		{
-			nextp = (*ownerp)->next;
-			if (nextp == 0)
-			{
-				nextp = *ownerp;
-				*ownerp = 0;
-			}
+			if (prevp != NULL)
+				prevp->next = nextp;
 			else
-			{
-				**ownerp = *nextp;
-			}
-			DELETE(nextp->owner);
-			DELETE(nextp);
+				q->owners = nextp;
+			DELETE(ownerp->owner);
+			DELETE(ownerp);
 		}
+		else
+			prevp = ownerp;
 	}
 }