| Message ID | 20190420191425.7d1dab82@luklap |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] net/colo-compare.c: Fix a crash in COLO Primary. | expand |
On 4/20/19 7:14 PM, Lukas Straub wrote: > From: Lukas Straub <lukasstraub2@web.de> > Because event_unhandled_count may be accessed concurrently, it needs > to be protected by taking the lock. However the assert is outside the > lock, probably causing it to read garbage and aborting Qemu erroneously. > > The Bug only happens when running Qemu in COLO mode. > > This Patch fixes the following bug: https://bugs.launchpad.net/qemu/+bug/1824622 > > Signed-off-by: Lukas Straub <lukasstraub2@web.de> > --- > net/colo-compare.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/net/colo-compare.c b/net/colo-compare.c > index bf10526f05..fcb491121b 100644 > --- a/net/colo-compare.c > +++ b/net/colo-compare.c > @@ -813,9 +813,8 @@ static void colo_compare_handle_event(void *opaque) > break; > } > > - assert(event_unhandled_count > 0); > - > qemu_mutex_lock(&event_mtx); > + assert(event_unhandled_count > 0); > event_unhandled_count--; > qemu_cond_broadcast(&event_complete_cond); > qemu_mutex_unlock(&event_mtx); > -- > 2.20.1 > > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> -----Original Message----- > From: Lukas Straub [mailto:lukasstraub2@web.de] > Sent: Sunday, April 21, 2019 1:14 AM > To: qemu-devel@nongnu.org > Cc: Zhang, Chen <chen.zhang@intel.com> > Subject: [PATCH v2] net/colo-compare.c: Fix a crash in COLO Primary. > > From: Lukas Straub <lukasstraub2@web.de> Because event_unhandled_count > may be accessed concurrently, it needs to be protected by taking the lock. > However the assert is outside the lock, probably causing it to read garbage and > aborting Qemu erroneously. > > The Bug only happens when running Qemu in COLO mode. > > This Patch fixes the following bug: > https://bugs.launchpad.net/qemu/+bug/1824622 > > Signed-off-by: Lukas Straub <lukasstraub2@web.de> Looks good for me. Reviewed-by: Zhang Chen <chen.zhang@intel.com> Thanks Zhang Chen > --- > net/colo-compare.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/net/colo-compare.c b/net/colo-compare.c index > bf10526f05..fcb491121b 100644 > --- a/net/colo-compare.c > +++ b/net/colo-compare.c > @@ -813,9 +813,8 @@ static void colo_compare_handle_event(void *opaque) > break; > } > > - assert(event_unhandled_count > 0); > - > qemu_mutex_lock(&event_mtx); > + assert(event_unhandled_count > 0); > event_unhandled_count--; > qemu_cond_broadcast(&event_complete_cond); > qemu_mutex_unlock(&event_mtx); > -- > 2.20.1
On Sat, 20 Apr 2019 19:14:25 +0200 Lukas Straub <lukasstraub2@web.de> wrote: > From: Lukas Straub <lukasstraub2@web.de> > Because event_unhandled_count may be accessed concurrently, it needs > to be protected by taking the lock. However the assert is outside the > lock, probably causing it to read garbage and aborting Qemu > erroneously. > > The Bug only happens when running Qemu in COLO mode. > > This Patch fixes the following bug: > https://bugs.launchpad.net/qemu/+bug/1824622 > > Signed-off-by: Lukas Straub <lukasstraub2@web.de> > --- > net/colo-compare.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/net/colo-compare.c b/net/colo-compare.c > index bf10526f05..fcb491121b 100644 > --- a/net/colo-compare.c > +++ b/net/colo-compare.c > @@ -813,9 +813,8 @@ static void colo_compare_handle_event(void > *opaque) break; > } > > - assert(event_unhandled_count > 0); > - > qemu_mutex_lock(&event_mtx); > + assert(event_unhandled_count > 0); > event_unhandled_count--; > qemu_cond_broadcast(&event_complete_cond); > qemu_mutex_unlock(&event_mtx); Ping. Regards, Lukas Straub
Cc'ing Paolo & Stefan On 4/20/19 7:14 PM, Lukas Straub wrote: > From: Lukas Straub <lukasstraub2@web.de> > Because event_unhandled_count may be accessed concurrently, it needs > to be protected by taking the lock. However the assert is outside the > lock, probably causing it to read garbage and aborting Qemu erroneously. > > The Bug only happens when running Qemu in COLO mode. > > This Patch fixes the following bug: https://bugs.launchpad.net/qemu/+bug/1824622 > > Signed-off-by: Lukas Straub <lukasstraub2@web.de> > --- > net/colo-compare.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/net/colo-compare.c b/net/colo-compare.c > index bf10526f05..fcb491121b 100644 > --- a/net/colo-compare.c > +++ b/net/colo-compare.c > @@ -813,9 +813,8 @@ static void colo_compare_handle_event(void *opaque) > break; > } > > - assert(event_unhandled_count > 0); > - > qemu_mutex_lock(&event_mtx); > + assert(event_unhandled_count > 0); > event_unhandled_count--; > qemu_cond_broadcast(&event_complete_cond); > qemu_mutex_unlock(&event_mtx); > -- > 2.20.1 > >
On 2019/5/6 下午6:32, Lukas Straub wrote: > On Sat, 20 Apr 2019 19:14:25 +0200 > Lukas Straub <lukasstraub2@web.de> wrote: > >> From: Lukas Straub <lukasstraub2@web.de> >> Because event_unhandled_count may be accessed concurrently, it needs >> to be protected by taking the lock. However the assert is outside the >> lock, probably causing it to read garbage and aborting Qemu >> erroneously. >> >> The Bug only happens when running Qemu in COLO mode. >> >> This Patch fixes the following bug: >> https://bugs.launchpad.net/qemu/+bug/1824622 >> >> Signed-off-by: Lukas Straub <lukasstraub2@web.de> >> --- >> net/colo-compare.c | 3 +-- >> 1 file changed, 1 insertion(+), 2 deletions(-) >> >> diff --git a/net/colo-compare.c b/net/colo-compare.c >> index bf10526f05..fcb491121b 100644 >> --- a/net/colo-compare.c >> +++ b/net/colo-compare.c >> @@ -813,9 +813,8 @@ static void colo_compare_handle_event(void >> *opaque) break; >> } >> >> - assert(event_unhandled_count > 0); >> - >> qemu_mutex_lock(&event_mtx); >> + assert(event_unhandled_count > 0); >> event_unhandled_count--; >> qemu_cond_broadcast(&event_complete_cond); >> qemu_mutex_unlock(&event_mtx); > Ping. > > Regards, > Lukas Straub Applied. Thanks >
diff --git a/net/colo-compare.c b/net/colo-compare.c index bf10526f05..fcb491121b 100644 --- a/net/colo-compare.c +++ b/net/colo-compare.c @@ -813,9 +813,8 @@ static void colo_compare_handle_event(void *opaque) break; } - assert(event_unhandled_count > 0); - qemu_mutex_lock(&event_mtx); + assert(event_unhandled_count > 0); event_unhandled_count--; qemu_cond_broadcast(&event_complete_cond); qemu_mutex_unlock(&event_mtx);