From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 01E43CAC599 for ; Wed, 17 Sep 2025 13:19:03 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1uys3V-0008LE-3r; Wed, 17 Sep 2025 09:18:44 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1uys3C-00086H-6J for qemu-devel@nongnu.org; Wed, 17 Sep 2025 09:18:25 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1uys39-0000jP-6X for qemu-devel@nongnu.org; Wed, 17 Sep 2025 09:18:21 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1758115098; h=from:from:reply-to: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: in-reply-to:in-reply-to:references:references; bh=Qdcwf8s9HmUuLp1IBeZbjsBqwFaRhrktluqh5mPF+QI=; b=iG9Z5FXkmXbln6dwAeO+lz45+Ci6y+a1YYmaolqTIsoDqPHwkt98t8i8ajJeCL//+HA8Hz 83SLNcmLM3mHAkgcgiBGxsoXUA6u0jJR2mjz/Xw5eaxwiqNcJ+HhRXHWaApNNxd4pl4B91 af84N4/dyem91UakgnIycn1cj1m8CH8= Received: from mx-prod-mc-08.mail-002.prod.us-west-2.aws.redhat.com (ec2-35-165-154-97.us-west-2.compute.amazonaws.com [35.165.154.97]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-624-ZAwOnfOcOMO9PEQblHgkzQ-1; Wed, 17 Sep 2025 09:18:14 -0400 X-MC-Unique: ZAwOnfOcOMO9PEQblHgkzQ-1 X-Mimecast-MFC-AGG-ID: ZAwOnfOcOMO9PEQblHgkzQ_1758115081 Received: from mx-prod-int-06.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-06.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.93]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-08.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id A350A18002D6; Wed, 17 Sep 2025 13:17:58 +0000 (UTC) Received: from redhat.com (unknown [10.42.28.195]) by mx-prod-int-06.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 8FCDC180044F; Wed, 17 Sep 2025 13:17:39 +0000 (UTC) Date: Wed, 17 Sep 2025 14:17:35 +0100 From: Daniel =?utf-8?B?UC4gQmVycmFuZ8Op?= To: Akihiko Odaki Cc: qemu-devel@nongnu.org, Alex Williamson , =?utf-8?Q?C=C3=A9dric?= Le Goater , Paolo Bonzini , Eduardo Habkost , Peter Xu , David Hildenbrand , Philippe =?utf-8?Q?Mathieu-Daud=C3=A9?= , Richard Henderson , Helge Deller , =?utf-8?Q?Marc-Andr=C3=A9?= Lureau , "Michael S. Tsirkin" , Gerd Hoffmann , John Snow , qemu-block@nongnu.org, Keith Busch , Klaus Jensen , Jesper Devantier , Marcel Apfelbaum , Nicholas Piggin , qemu-ppc@nongnu.org, John Levon , Thanos Makatos , Yanan Wang , BALATON Zoltan , Jiaxun Yang , Daniel Henrique Barboza , David Gibson , Harsh Prateek Bora , Alexey Kardashevskiy , Alex =?utf-8?Q?Benn=C3=A9e?= , Fabiano Rosas , Thomas Huth , Laurent Vivier , Peter Maydell , Aurelien Jarno , Aleksandar Rikalo , Max Filippov , =?utf-8?B?SGVydsOp?= Poussineau , Mark Cave-Ayland , Artyom Tarasenko , Alistair Francis , "Maciej S. Szmigiero" , Bin Meng , Stefano Stabellini , Anthony PERARD , Paul Durrant , "Edgar E. Iglesias" , xen-devel@lists.xenproject.org Subject: Re: [PATCH v3 0/7] Do not unparent in instance_finalize() Message-ID: References: <20250917-use-v3-0-72c2a6887c6c@rsg.ci.i.u-tokyo.ac.jp> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/2.2.14 (2025-02-20) X-Scanned-By: MIMEDefang 3.4.1 on 10.30.177.93 Received-SPF: pass client-ip=170.10.133.124; envelope-from=berrange@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H5=0.001, RCVD_IN_MSPIKE_WL=0.001, RCVD_IN_VALIDITY_RPBL_BLOCKED=0.001, RCVD_IN_VALIDITY_SAFE_BLOCKED=0.001, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001 autolearn=ham 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: , Reply-To: Daniel =?utf-8?B?UC4gQmVycmFuZ8Op?= Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org On Wed, Sep 17, 2025 at 09:24:04PM +0900, Akihiko Odaki wrote: > On 2025/09/17 20:57, Daniel P. Berrangé wrote: > > On Wed, Sep 17, 2025 at 07:13:25PM +0900, Akihiko Odaki wrote: > > > Based-on: > > > ("[PATCH v2 00/14] hw/pci-host/raven clean ups") > > > > > > Supersedes: <20240829-memory-v1-1-ac07af2f4fa5@daynix.com> > > > ("[PATCH] docs/devel: Prohibit calling object_unparent() for memory region") > > > > > > Children are automatically unparented so manually unparenting is > > > unnecessary. > > > > Where is automatic unparenting you're referring to being done ? > > > > > Worse, automatic unparenting happens before the instance_finalize() > > > callback of the parent gets called, so object_unparent() calls in > > > the callback will refer to objects that are already unparented, which > > > is semantically incorrect. > > > > IIUC, object_property_add_child will acquire a reference on > > the child, and object_property_del_child (and thus > > object_unparent) will release that reference. > > > > The 'object_finalize' method, and thus 'instance_finalize' > > callback, won't be invoked until the last reference is > > dropped on the object in question. > > > > IOW, it should be impossible for 'object_finalize' to ever > > run, as long as the child has a parent set. > > > > So if we're in the 'finalize' then 'object_unparent' must > > be a no-op as the child must already have no references > > held and thus no parent. > > > > IOW, the reason to remove 'object_unparent' calls from > > finalize is surely because they do nothing at all, > > rather than this talk about callbacks being run at the > > wrong time ? > > This patch series deals with the situation where the parent calls > object_unparent() in its instance_finalize() callback. The process of > finalization looks like as follows: > > 1. The parent's reference count reaches to zero. Please note that there can > be remaining children that are referenced by the parent at this point. > > 2. object_finalize() is called. > > 2a. object_property_del_all() is called and the parent releases references > to its children. This is what I referred as "automatic unparenting". The > children without any other references will be finalized here. > > 2b. instance_finalize() is called. Past children may be already finalized, > and calling object_unparent() here will cause dereferencing finalized > objects in that case, which should be avoided. Oh, so these object_unparent calls run by the parent, against the child in fact use-after-free flaws. This is driven by the parent keeping hold of explicit pointers to the child (MemoryRegion), without also holding its own reference, and these pointers are invalidated when the parent<->child property is deleted. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|