Patchwork acl: Fix use after free in qemu_acl_reset()

login
register
mail settings
Submitter Markus Armbruster
Date Oct. 28, 2011, 3:07 p.m.
Message ID <1319814422-17952-1-git-send-email-armbru@redhat.com>
Download mbox | patch
Permalink /patch/122419/
State New
Headers show

Comments

Markus Armbruster - Oct. 28, 2011, 3:07 p.m.
Reproducer:

    $ MALLOC_PERTURB_=234 qemu-system-x86_64 -vnc :0,acl,sasl [...]
    QEMU 0.15.50 monitor - type 'help' for more information
    (qemu) acl_add vnc.username fred allow
    acl: added rule at position 1
    (qemu) acl_reset vnc.username
    Segmentation fault (core dumped)

Spotted by Coverity.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 acl.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
Stefan Hajnoczi - Oct. 31, 2011, 11:55 a.m.
On Fri, Oct 28, 2011 at 05:07:02PM +0200, Markus Armbruster wrote:
> Reproducer:
> 
>     $ MALLOC_PERTURB_=234 qemu-system-x86_64 -vnc :0,acl,sasl [...]
>     QEMU 0.15.50 monitor - type 'help' for more information
>     (qemu) acl_add vnc.username fred allow
>     acl: added rule at position 1
>     (qemu) acl_reset vnc.username
>     Segmentation fault (core dumped)
> 
> Spotted by Coverity.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  acl.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)

Thanks, applied to the trivial patches -next tree:
http://repo.or.cz/w/qemu/stefanha.git/shortlog/refs/heads/trivial-patches-next

Stefan

Patch

diff --git a/acl.c b/acl.c
index 0654f38..e840b9b 100644
--- a/acl.c
+++ b/acl.c
@@ -95,13 +95,13 @@  int qemu_acl_party_is_allowed(qemu_acl *acl,
 
 void qemu_acl_reset(qemu_acl *acl)
 {
-    qemu_acl_entry *entry;
+    qemu_acl_entry *entry, *next_entry;
 
     /* Put back to deny by default, so there is no window
      * of "open access" while the user re-initializes the
      * access control list */
     acl->defaultDeny = 1;
-    QTAILQ_FOREACH(entry, &acl->entries, next) {
+    QTAILQ_FOREACH_SAFE(entry, &acl->entries, next, next_entry) {
         QTAILQ_REMOVE(&acl->entries, entry, next);
         free(entry->match);
         free(entry);