[PATCH] compile-i386: fix use-after-free in func_cleanup()

[PATCH] compile-i386: fix use-after-free in func_cleanup()

[Xi Wang]

compile-i386 sometimes crashes due a use-after-free error.  Since
f->pseudo_list is freed first, which invalidates some atom->op* in
f->atom_list.  Further checks like `atom->op1->flags & STOR_WANTS_FREE'
will read garbage, which may lead to a double free.

This patch switches the cleanup order and frees f->atom_list first.
Those marked as STOR_WANTS_FREE won't appear in f->pseudo_list.

Signed-off-by: Xi Wang <xi.wang@gmail.com>
---
 compile-i386.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/compile-i386.c b/compile-i386.c
index da3ee49..b470952 100644
--- a/compile-i386.c
+++ b/compile-i386.c
@@ -761,10 +761,6 @@ static void func_cleanup(struct function *f)
 	struct storage *stor;
 	struct atom *atom;
 
-	FOR_EACH_PTR(f->pseudo_list, stor) {
-		free(stor);
-	} END_FOR_EACH_PTR(stor);

Re: [PATCH] compile-i386: fix use-after-free in func_cleanup()

[Jeff Garzik]

On Fri, Jun 1, 2012 at 9:55 PM, Christopher Li <sparse@chrisli.org> wrote:
> On Fri, Jun 1, 2012 at 9:22 AM, Xi Wang <xi.wang@gmail.com> wrote:
>> compile-i386 sometimes crashes due a use-after-free error.  Since
>> f->pseudo_list is freed first, which invalidates some atom->op* in
>> f->atom_list.  Further checks like `atom->op1->flags & STOR_WANTS_FREE'
>> will read garbage, which may lead to a double free.
>>
>> This patch switches the cleanup order and frees f->atom_list first.
>> Those marked as STOR_WANTS_FREE won't appear in f->pseudo_list.
>
> Seems make sense. I will apply if Jeff don't have any objections.

If the patch makes sense, go for it.  I'm on vacation in France until
June 8, the original patch is not in my inbox, and I do not have
readily available Internet connectivity besides... :)
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] compile-i386: fix use-after-free in func_cleanup()

[Pekka Enberg]

On Mon, Jun 4, 2012 at 12:42 PM, Jeff Garzik <jgarzik@pobox.com> wrote:
> On Fri, Jun 1, 2012 at 9:55 PM, Christopher Li <sparse@chrisli.org> wrote:
>> On Fri, Jun 1, 2012 at 9:22 AM, Xi Wang <xi.wang@gmail.com> wrote:
>>> compile-i386 sometimes crashes due a use-after-free error.  Since
>>> f->pseudo_list is freed first, which invalidates some atom->op* in
>>> f->atom_list.  Further checks like `atom->op1->flags & STOR_WANTS_FREE'
>>> will read garbage, which may lead to a double free.
>>>
>>> This patch switches the cleanup order and frees f->atom_list first.
>>> Those marked as STOR_WANTS_FREE won't appear in f->pseudo_list.
>>
>> Seems make sense. I will apply if Jeff don't have any objections.
>
> If the patch makes sense, go for it.  I'm on vacation in France until
> June 8, the original patch is not in my inbox, and I do not have
> readily available Internet connectivity besides... :)

Looks good to me.

Reviewed-by: Pekka Enberg <penberg@kernel.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html