[Spice-devel] [PATCH] server/red_worker: fix use after free for listeners

[Alon Levy]

On Tue, Mar 06, 2012 at 06:07:06PM +0200, Alon Levy wrote:
> On Tue, Mar 06, 2012 at 03:56:30PM +0100, Hans de Goede wrote:
> > ACK!
> > 
> 
> otoh I'm not too happy with perf top:

but it isn't introduced by this commit, so I'll push.

> 
> (this accounts for 5% of samples, i.e. cpu):
> 
>          :                for (i = 0; i < MAX_EVENT_SOURCES; i++) {
>     0.21 :           3fc49:       movl   $0x0,-0x4(%rbp)
>     0.28 :           3fc50:       jmpq   3fd1f <red_worker_main+0x28e>
>          :                    struct pollfd *pfd = worker.poll_fds + i;
>     3.77 :           3fc55:       mov    -0x4(%rbp),%eax
>     0.37 :           3fc58:       movslq %eax,%rdx
>     0.00 :           3fc5b:       lea    -0x1d76c0(%rbp),%rax
>     0.00 :           3fc62:       add    $0xa,%rdx
>     3.98 :           3fc66:       shl    $0x3,%rdx
>     0.15 :           3fc6a:       add    %rdx,%rax
>     0.46 :           3fc6d:       mov    %rax,-0x18(%rbp)
>          :                    if (pfd->revents) {
>     0.70 :           3fc71:       mov    -0x18(%rbp),%rax
>     8.00 :           3fc75:       movzwl 0x6(%rax),%eax
>    12.69 :           3fc79:       test   %ax,%ax
>     0.61 :           3fc7c:       je     3fd1b <red_worker_main+0x28a>
> 
> > On 03/06/2012 03:50 PM, Alon Levy wrote:
> > >This fixes a core dumped observed once by repeated migration. So far 100
> > >migrations and no recurrence.
> > >
> > >Core was generated by `/home/alon/spice/upstream/bin/qemu-system-x86_64 --enable-kvm -qmp unix:/tmp/mi'.
> > >Program terminated with signal 11, Segmentation fault.
> > >11197	                if (evt_listener&&  evt_listener->refs>  1) {
> > >Missing separate debuginfos, use: debuginfo-install bluez-libs-4.98-3.fc17.x86_64 brlapi-0.5.6-4.fc17.x86_64 bzip2-libs-1.0.6-4.fc17.x86_64 cryptopp-5.6.1-6.fc17.x86_64 keyutils-libs-1.5.5-2.fc17.x86_64 libssh2-1.4.0-1.fc17.x86_64 nss-softokn-freebl-3.13.1-20.fc17.x86_64 xen-libs-4.1.2-11.fc17.x86_64 xz-libs-5.1.1-2alpha.fc17.x86_64
> > >(gdb) bt
> > >(gdb) l
> > >11192	        for (i = 0; i<  MAX_EVENT_SOURCES; i++) {
> > >11193	            struct pollfd *pfd = worker.poll_fds + i;
> > >11194	            if (pfd->revents) {
> > >11195	                EventListener *evt_listener = worker.listeners[i];
> > >11196
> > >11197	                if (evt_listener&&  evt_listener->refs>  1) {
> > >11198	                    evt_listener->action(evt_listener, pfd);
> > >11199	                    if (--evt_listener->refs) {
> > >11200	                        continue;
> > >11201	                    }
> > >(gdb) p evt_listener
> > >$1 = (EventListener *) 0x7f15a9a5d1e0
> > >(gdb) p *evt_listener
> > >Cannot access memory at address 0x7f15a9a5d1e0
> > >(gdb) p i
> > >$2 = 2
> > >(gdb) p worker.listeners
> > >$3 = {0x7f15bc832520, 0x7f15a406e1a0, 0x7f15a9a5d1e0, 0x0<repeats 17 times>}
> > >---
> > >  server/red_worker.c |    3 ++-
> > >  1 files changed, 2 insertions(+), 1 deletions(-)
> > >
> > >diff --git a/server/red_worker.c b/server/red_worker.c
> > >index e88dbc0..a2f31c0 100644
> > >--- a/server/red_worker.c
> > >+++ b/server/red_worker.c
> > >@@ -11194,7 +11194,7 @@ void *red_worker_main(void *arg)
> > >              if (pfd->revents) {
> > >                  EventListener *evt_listener = worker.listeners[i];
> > >
> > >-                if (evt_listener->refs>  1) {
> > >+                if (evt_listener&&  evt_listener->refs>  1) {
> > >                      evt_listener->action(evt_listener, pfd);
> > >                      if (--evt_listener->refs) {
> > >                          continue;
> > >@@ -11202,6 +11202,7 @@ void *red_worker_main(void *arg)
> > >                  }
> > >                  red_printf("freeing event listener");
> > >                  evt_listener->free(evt_listener);
> > >+                worker.listeners[i] = NULL;
> > >              }
> > >          }
> > >
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/spice-devel
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel