From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49638) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1agZQ8-0001os-La for qemu-devel@nongnu.org; Thu, 17 Mar 2016 11:01:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1agZQ4-0000Ck-Jk for qemu-devel@nongnu.org; Thu, 17 Mar 2016 11:01:04 -0400 Date: Thu, 17 Mar 2016 15:00:57 +0000 From: Stefan Hajnoczi Message-ID: <20160317150057.GP14062@stefanha-x1.localdomain> References: <1458123018-18651-1-git-send-email-famz@redhat.com> <1458123018-18651-5-git-send-email-famz@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="xXygN3QAmJYWdGtb" Content-Disposition: inline In-Reply-To: <1458123018-18651-5-git-send-email-famz@redhat.com> Subject: Re: [Qemu-devel] [Qemu-block] [PATCH 4/4] virtio-blk: Clean up start/stop with mutex and BH List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng Cc: Kevin Wolf , qemu-block@nongnu.org, "Michael S. Tsirkin" , qemu-devel@nongnu.org, tubo@linux.vnet.ibm.com, Stefan Hajnoczi , cornelia.huck@de.ibm.com, pbonzini@redhat.com, borntraeger@de.ibm.com --xXygN3QAmJYWdGtb Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Wed, Mar 16, 2016 at 06:10:18PM +0800, Fam Zheng wrote: > + data = g_new(VirtIOBlockStartData, 1); > + data->vblk = vblk; > + data->bh = aio_bh_new(s->ctx, virtio_blk_data_plane_start_bh_cb, data); > + qemu_bh_schedule(data->bh); > + qemu_mutex_unlock(&s->start_stop_lock); > return; This BH usage pattern is dangerous: 1. The BH is created and scheduled. 2. Before the BH executes the device is unrealized. 3. The data->bh pointer is inaccessible so we have a dangling BH that will access vblk after vblk has been freed. In some cases it can be safe but I don't see why the pattern is safe in this case. Either the BH needs to hold some sort of reference to keep vblk alive, or vblk needs to know about pending BHs so they can be deleted. --xXygN3QAmJYWdGtb Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJW6sapAAoJEJykq7OBq3PIrBoH/i4gFzTcnG9imxYVAvzTGPFT Jx3xXktJl+QrIFdLfY78xucoulz0I8089tOYJYZbTWC1HyNd/C/dJtQ/q7sjgB9N Z9M2Cu+kJTviY+LjzoUIPXN6fEsME7clvrorjrmsMCBoAgmCVH/It4yRluAoSJ53 05P3Yu8Fnqj20aYwzdeYsstTEm8sIf+YhnVVbbcqTUdZI+s7G1/CCxm3szHN0Qk2 VatdCI9yQ+iw7FihL3vD3ZAgHpWBhoXFJt1tdjS3gVXkmgo4XDdYp+Sq20p3Dfq5 AMpnWieUP6fxrJNptlwG4z+s2K1h54A8GfeP6T6SYS2aAiTyAR1HlN4R9CDTLjk= =3wc8 -----END PGP SIGNATURE----- --xXygN3QAmJYWdGtb--