RHEL Engineering is moving the tracking of its product development work on RHEL 6 through RHEL 9 to Red Hat Jira (issues.redhat.com). If you're a Red Hat customer, please continue to file support cases via the Red Hat customer portal. If you're not, please head to the "RHEL project" in Red Hat Jira and file new tickets here. Individual Bugzilla bugs in the statuses "NEW", "ASSIGNED", and "POST" are being migrated throughout September 2023. Bugs of Red Hat partners with an assigned Engineering Partner Manager (EPM) are migrated in late September as per pre-agreed dates. Bugs against components "kernel", "kernel-rt", and "kpatch" are only migrated if still in "NEW" or "ASSIGNED". If you cannot log in to RH Jira, please consult article #7032570. That failing, please send an e-mail to the RH Jira admins at rh-issues@redhat.com to troubleshoot your issue as a user management inquiry. The email creates a ServiceNow ticket with Red Hat. Individual Bugzilla bugs that are migrated will be moved to status "CLOSED", resolution "MIGRATED", and set with "MigratedToJIRA" in "Keywords". The link to the successor Jira issue will be found under "Links", have a little "two-footprint" icon next to it, and direct you to the "RHEL project" in Red Hat Jira (issue links are of type "https://issues.redhat.com/browse/RHEL-XXXX", where "X" is a digit). This same link will be available in a blue banner at the top of the page informing you that that bug has been migrated.
Bug 1116728 - Backport qemu_bh_schedule() race condition fix
Summary: Backport qemu_bh_schedule() race condition fix
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Linux 7
Classification: Red Hat
Component: qemu-kvm
Version: 7.1
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: rc
: ---
Assignee: John Snow
QA Contact: Virtualization Bugs
URL:
Whiteboard:
Depends On:
Blocks: 1116729
TreeView+ depends on / blocked
 
Reported: 2014-07-07 07:50 UTC by Stefan Hajnoczi
Modified: 2015-03-05 08:10 UTC (History)
9 users (show)

Fixed In Version: qemu-kvm-1.5.3-67.el7
Doc Type: Bug Fix
Doc Text:
Clone Of:
: 1116729 (view as bug list)
Environment:
Last Closed: 2015-03-05 08:10:37 UTC
Target Upstream Version:
Embargoed:

Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Red Hat Product Errata RHSA-2015:0349 0 normal SHIPPED_LIVE Important: qemu-kvm security, bug fix, and enhancement update 2015-03-05 12:27:34 UTC

Description Stefan Hajnoczi 2014-07-07 07:50:27 UTC 
There is a race condition in qemu_bh_schedule() that was fixed upstream:

commit 924fe1293c3e7a3c787bbdfb351e7f168caee3e9
Author: Stefan Hajnoczi <stefanha>
Date:   Tue Jun 3 11:21:01 2014 +0200

    aio: fix qemu_bh_schedule() bh->ctx race condition
    
    qemu_bh_schedule() is supposed to be thread-safe at least the first time
    it is called.  Unfortunately this is not quite true:
    
      bh->scheduled = 1;
      aio_notify(bh->ctx);
    
    Since another thread may run the BH callback once it has been scheduled,
    there is a race condition if the callback frees the BH before
    aio_notify(bh->ctx) has a chance to run.
    
    Reported-by: Stefan Priebe <s.priebe>
    Signed-off-by: Stefan Hajnoczi <stefanha>
    Reviewed-by: Paolo Bonzini <pbonzini>
    Tested-by: Stefan Priebe <s.priebe>

Upstream both block/rbd.c and block/gluster.c were affected by this race condition and users reported periodic crashes.  In rhel7 (based on QEMU 1.5.3) these block drivers use a pipe to signal request completion instead of QEMUBH.  Therefore they are unaffected in RHEL7.

This fix should still be backported since it is hard to detect the presence of this bug and future RHEL7 backports may bring in code that depends on qemu_bh_schedule()'s thread-safety.
Comment 2 FuXiangChun 2014-07-22 10:04:15 UTC 
Stefan,
would you please provide QE an effective way or reproducer to reproduce this bug? otherwise, QE don't know how to trigger/verify it.  Thanks!
Comment 3 Stefan Hajnoczi 2014-07-23 14:14:45 UTC 
(In reply to FuXiangChun from comment #2)
> would you please provide QE an effective way or reproducer to reproduce this
> bug? otherwise, QE don't know how to trigger/verify it.  Thanks!

There is no good test available for this race condition.  I think we'll have to rely on code review for this one.
Comment 4 Jeff Nelson 2014-08-08 16:54:56 UTC 
Fix included in qemu-kvm-1.5.3-67.el7
Comment 5 FuXiangChun 2014-08-13 06:01:25 UTC 
Verify with qemu-kvm-1.5.3-67.el7.src.rpm

# rpm -qpi qemu-kvm-1.5.3-67.el7.src.rpm --changelog|grep 1116728
- kvm-aio-fix-qemu_bh_schedule-bh-ctx-race-condition.patch [bz#1116728]

check kvm-aio-fix-qemu_bh_schedule-bh-ctx-race-condition.patch

diff --git a/async.c b/async.c
index 5ce3633..d7ec1ea 100644
--- a/async.c
+++ b/async.c
@@ -117,15 +117,21 @@ void qemu_bh_schedule_idle(QEMUBH *bh)

 void qemu_bh_schedule(QEMUBH *bh)
 {
+    AioContext *ctx;
+
     if (bh->scheduled)
         return;
+    ctx = bh->ctx;
     bh->idle = 0;
-    /* Make sure that idle & any writes needed by the callback are done
-     * before the locations are read in the aio_bh_poll.
+    /* Make sure that:
+     * 1. idle & any writes needed by the callback are done before the
+     *    locations are read in the aio_bh_poll.
+     * 2. ctx is loaded before scheduled is set and the callback has a chance
+     *    to execute.
      */
-    smp_wmb();
+    smp_mb();
     bh->scheduled = 1;
-    aio_notify(bh->ctx);
+    aio_notify(ctx);
 }

# rpm -qpi qemu-kvm-rhev-2.1.0-1.el7.src.rpm --changelog|grep 1116728
nothing

Stefan,
According to this test result. This bug isn't fixed for qemu-2.1.  It is fixed for qemu-kvm-1.5.3-67.el7. Is this bug verified?
Comment 6 Stefan Hajnoczi 2014-08-13 14:09:37 UTC 
(In reply to FuXiangChun from comment #5)
> Verify with qemu-kvm-1.5.3-67.el7.src.rpm
> 
> # rpm -qpi qemu-kvm-1.5.3-67.el7.src.rpm --changelog|grep 1116728
> - kvm-aio-fix-qemu_bh_schedule-bh-ctx-race-condition.patch [bz#1116728]
> 
> check kvm-aio-fix-qemu_bh_schedule-bh-ctx-race-condition.patch
> 
> diff --git a/async.c b/async.c
> index 5ce3633..d7ec1ea 100644
> --- a/async.c
> +++ b/async.c
> @@ -117,15 +117,21 @@ void qemu_bh_schedule_idle(QEMUBH *bh)
> 
>  void qemu_bh_schedule(QEMUBH *bh)
>  {
> +    AioContext *ctx;
> +
>      if (bh->scheduled)
>          return;
> +    ctx = bh->ctx;
>      bh->idle = 0;
> -    /* Make sure that idle & any writes needed by the callback are done
> -     * before the locations are read in the aio_bh_poll.
> +    /* Make sure that:
> +     * 1. idle & any writes needed by the callback are done before the
> +     *    locations are read in the aio_bh_poll.
> +     * 2. ctx is loaded before scheduled is set and the callback has a
> chance
> +     *    to execute.
>       */
> -    smp_wmb();
> +    smp_mb();
>      bh->scheduled = 1;
> -    aio_notify(bh->ctx);
> +    aio_notify(ctx);
>  }
> 
> # rpm -qpi qemu-kvm-rhev-2.1.0-1.el7.src.rpm --changelog|grep 1116728
> nothing
> 
> Stefan,
> According to this test result. This bug isn't fixed for qemu-2.1.  It is
> fixed for qemu-kvm-1.5.3-67.el7. Is this bug verified?

The patch is part of QEMU 2.1 so no explicit backport was needed.  That's why you don't see 1116728 in the grep output.

If you want to double-check, look at async.c in the QEMU 2.1.0 source code used to build the qemu-kvm-rhev-2.1.0-1.el7 rpm.
Comment 8 FuXiangChun 2014-08-28 13:47:40 UTC 
verified this bug with qemu-kvm-rhev-2.1.0-2.el7.src.rpm

void qemu_bh_schedule(QEMUBH *bh)
{
    AioContext *ctx;

    if (bh->scheduled)
        return;
    ctx = bh->ctx;
    bh->idle = 0;
    /* Make sure that:
     * 1. idle & any writes needed by the callback are done before the
     *    locations are read in the aio_bh_poll.
     * 2. ctx is loaded before scheduled is set and the callback has a chance
     *    to execute.
     */
    smp_mb();
    bh->scheduled = 1;
    aio_notify(ctx);
}

according to https://bugzilla.redhat.com/show_bug.cgi?id=1116729#c6 and comment 6 as above.

Base on this test result. This bug is fixed on qemu-kvm-rhev-2.1.0-2.el7.src.rpm. but it isn't fixed on qemu-kvm-1.5.3-67.el7
Comment 9 juzhang 2014-09-01 04:50:43 UTC 
(In reply to FuXiangChun from comment #8)
> verified this bug with qemu-kvm-rhev-2.1.0-2.el7.src.rpm
> 
> void qemu_bh_schedule(QEMUBH *bh)
> {
>     AioContext *ctx;
> 
>     if (bh->scheduled)
>         return;
>     ctx = bh->ctx;
>     bh->idle = 0;
>     /* Make sure that:
>      * 1. idle & any writes needed by the callback are done before the
>      *    locations are read in the aio_bh_poll.
>      * 2. ctx is loaded before scheduled is set and the callback has a chance
>      *    to execute.
>      */
>     smp_mb();
>     bh->scheduled = 1;
>     aio_notify(ctx);
> }
> 
> according to https://bugzilla.redhat.com/show_bug.cgi?id=1116729#c6 and
> comment 6 as above.
> 
> Base on this test result. This bug is fixed on
> qemu-kvm-rhev-2.1.0-2.el7.src.rpm. but it isn't fixed on
> qemu-kvm-1.5.3-67.el7

qemu-kvm-rhev-2.1.0-2.el7.src.rpm and qemu-kvm-1.5.3-67.el7
Comment 12 errata-xmlrpc 2015-03-05 08:10:37 UTC 
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://rhn.redhat.com/errata/RHSA-2015-0349.html
Note You need to log in before you can comment on or make changes to this bug.