From nobody Fri May 3 18:49:31 2024 Delivered-To: importer@patchew.org Authentication-Results: mx.zohomail.com; dkim=fail; spf=pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mx.zohomail.com with SMTPS id 1639059944549269.5031407293093; Thu, 9 Dec 2021 06:25:44 -0800 (PST) Received: from localhost ([::1]:60332 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mvKMh-00083I-9g for importer@patchew.org; Thu, 09 Dec 2021 09:25:43 -0500 Received: from eggs.gnu.org ([209.51.188.92]:49760) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mvKKJ-0006TB-3a for qemu-devel@nongnu.org; Thu, 09 Dec 2021 09:23:17 -0500 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]:30033) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mvKKF-0002FL-7K for qemu-devel@nongnu.org; Thu, 09 Dec 2021 09:23:12 -0500 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-257-tF03HV_yNSindHJsRwHn7Q-1; Thu, 09 Dec 2021 09:23:07 -0500 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 57111100C623; Thu, 9 Dec 2021 14:23:06 +0000 (UTC) Received: from localhost (unknown [10.39.194.236]) by smtp.corp.redhat.com (Postfix) with ESMTP id EFE425D6CF; Thu, 9 Dec 2021 14:23:05 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1639059790; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=6HXk94Amc9Res95h63mWGwKBtpPj+cAGMHMrZFyisj8=; b=JGD0c2WjqmiKXO31yoI2DQQAj9jo5qt1GLgl09gtnvSHzW4e0kqtGTRX7Nk1bqacnxHU43 biogHOQITwXy7w0HgRXMtYKBMiz8/7SHaYhwqW/L5c26jEZ8d5jDdJrrp7jhpuzYrWXXpQ AWn+p65xAS3TKguQNVniQ0hKZvL9tQE= X-MC-Unique: tF03HV_yNSindHJsRwHn7Q-1 From: Stefan Hajnoczi To: qemu-devel@nongnu.org Subject: [RFC] block-backend: prevent dangling BDS pointer in blk_drain() Date: Thu, 9 Dec 2021 14:23:04 +0000 Message-Id: <20211209142304.381253-1-stefanha@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=stefanha@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: quoted-printable Received-SPF: pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Received-SPF: pass client-ip=170.10.129.124; envelope-from=stefanha@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -33 X-Spam_score: -3.4 X-Spam_bar: --- X-Spam_report: (-3.4 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.618, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=unavailable autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Hanna Reitz , Vladimir Sementsov-Ogievskiy , Stefan Hajnoczi , qemu-block@nongnu.org Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail-DKIM: fail (Header signature does not verify) X-ZM-MESSAGEID: 1639059945741100001 Content-Type: text/plain; charset="utf-8" The BlockBackend root child can change during bdrv_drained_begin() when aio_poll() is invoked. In fact the BlockDriverState can reach refcnt 0 and blk_drain() is left with a dangling BDS pointer. One example is scsi_device_purge_requests(), which calls blk_drain() to wait for in-flight requests to cancel. If the backup blockjob is active, then the BlockBackend root child is a temporary filter BDS owned by the blockjob. The blockjob can complete during bdrv_drained_begin() and the last reference to the BDS is released when the temporary filter node is removed. This results in a use-after-free when blk_drain() calls bdrv_drained_end(bs) on the dangling pointer. The general problem is that a function and its callers must not assume that bs is still valid across aio_poll(). Explicitly hold a reference to bs in blk_drain() to avoid the dangling pointer. Signed-off-by: Stefan Hajnoczi Reviewed-by: Vladimir Sementsov-Ogievskiy --- I found that BDS nodes are sometimes deleted with bs->quiesce_counter > 0 (at least when running "make check"), so it is currently not possible to put the bdrv_ref/unref() calls in bdrv_do_drained_begin() and bdrv_do_drained_end() because they will be unbalanced. That would have been a more general solution than only fixing blk_drain(). Any suggestions for a better fix? I think it's likely that more "dangling pointer across aio_poll()" problems exist :(. Here is the (hacky) reproducer: build/qemu-system-x86_64 \ -name 'avocado-vt-vm1' \ -sandbox on \ -machine q35,memory-backend=3Dmem-machine_mem \ -device pcie-root-port,id=3Dpcie-root-port-0,multifunction=3Don,bus=3D= pcie.0,addr=3D0x1,chassis=3D1 \ -device pcie-pci-bridge,id=3Dpcie-pci-bridge-0,addr=3D0x0,bus=3Dpcie-r= oot-port-0 \ -nodefaults \ -device VGA,bus=3Dpcie.0,addr=3D0x2 \ -m 1024 \ -object memory-backend-ram,size=3D1024M,id=3Dmem-machine_mem \ -smp 10,maxcpus=3D10,cores=3D5,threads=3D1,dies=3D1,sockets=3D2 \ -cpu 'Cascadelake-Server-noTSX',+kvm_pv_unhalt \ -chardev socket,wait=3Doff,server=3Don,id=3Dqmp_id_qmpmonitor1,path=3D= /tmp/qmp.sock \ -mon chardev=3Dqmp_id_qmpmonitor1,mode=3Dcontrol \ -chardev socket,wait=3Doff,server=3Don,id=3Dqmp_id_catch_monitor,path= =3D/tmp/catch_monitor.sock \ -mon chardev=3Dqmp_id_catch_monitor,mode=3Dcontrol \ -device pvpanic,ioport=3D0x505,id=3DidgKHYrQ \ -chardev socket,wait=3Doff,server=3Don,id=3Dchardev_serial0,path=3D/tm= p/serial.sock \ -device isa-serial,id=3Dserial0,chardev=3Dchardev_serial0 \ -chardev socket,id=3Dseabioslog_id_20211110-012521-TNCkxDmn,path=3D/tm= p/seabios.sock,server=3Don,wait=3Doff \ -device isa-debugcon,chardev=3Dseabioslog_id_20211110-012521-TNCkxDmn,= iobase=3D0x402 \ -device pcie-root-port,id=3Dpcie-root-port-2,port=3D0x2,addr=3D0x1.0x2= ,bus=3Dpcie.0,chassis=3D3 \ -device virtio-scsi-pci,id=3Dvirtio_scsi_pci0,bus=3Dpcie-root-port-2,a= ddr=3D0x0 \ -blockdev node-name=3Dfile_image1,driver=3Dfile,auto-read-only=3Don,di= scard=3Dunmap,aio=3Dthreads,filename=3Dtest.img,cache.direct=3Don,cache.no-= flush=3Doff \ -blockdev node-name=3Ddrive_image1,driver=3Draw,read-only=3Doff,cache.= direct=3Don,cache.no-flush=3Doff,file=3Dfile_image1 \ -device scsi-hd,id=3Dimage1,drive=3Ddrive_image1,write-cache=3Don \ -blockdev node-name=3Dfile_src1,driver=3Dfile,auto-read-only=3Don,disc= ard=3Dunmap,aio=3Dthreads,filename=3Dsr1.qcow2,cache.direct=3Don,cache.no-f= lush=3Doff \ -blockdev node-name=3Ddrive_src1,driver=3Dqcow2,read-only=3Doff,cache.= direct=3Don,cache.no-flush=3Doff,file=3Dfile_src1 \ -device scsi-hd,id=3Dsrc1,drive=3Ddrive_src1,write-cache=3Don \ -device pcie-root-port,id=3Dpcie-root-port-3,port=3D0x3,addr=3D0x1.0x3= ,bus=3Dpcie.0,chassis=3D4 \ -device virtio-net-pci,mac=3D9a:11:64:b0:5d:a8,id=3DidxnEEYY,netdev=3D= idBjpylo,bus=3Dpcie-root-port-3,addr=3D0x0 \ -netdev user,id=3DidBjpylo \ -vnc :0 \ -rtc base=3Dutc,clock=3Dhost,driftfix=3Dslew \ -boot menu=3Doff,order=3Dcdn,once=3Dc,strict=3Doff \ -enable-kvm \ -device pcie-root-port,id=3Dpcie_extra_root_port_0,multifunction=3Don,= bus=3Dpcie.0,addr=3D0x3,chassis=3D5 & sleep 8 # delay for VM startup and socket creation nc -U /tmp/qmp.sock <