From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6346E22154B for ; Sat, 20 Dec 2025 18:01:36 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1766253696; cv=none; b=i41d+jAmKPcU9LvRdzk9vERX7Y/KJa/aBQE5YIBF8GZARwOP7YOpuW4+on/JMl+zIeWpD1WOSA80QTdelMgT0XInMgd0xwOQDxav2x0/6OsavHKy4STKiFUTqe09TD9TJmRpheINvdHMZPiqSV6whufuBy+cpxvB+F3wN402FSQ= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1766253696; c=relaxed/simple; bh=rNegeb1NXvndOVs5fOFiH3UyPRPJyzu8EuRv+VEVlfQ=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=bQcb3jP30UotzIR33eY8d8Qn2BfnT6v2IIRU3V4NWE4xY6Cracdt8QKos5axx1AiKOVD/HJCvlKQTErBDgGOBNhwtNtGArxYmQ38T8wM9KGw3I5R8ZDkniNCi1b3NbL5S1/lJ+NlrJvFqZZQUrCEh99jpWz8eVRDVdaPcoaHxPU= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=LvVMSUf8; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="LvVMSUf8" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 24C20C4CEF5; Sat, 20 Dec 2025 18:01:33 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1766253695; bh=rNegeb1NXvndOVs5fOFiH3UyPRPJyzu8EuRv+VEVlfQ=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=LvVMSUf83ppKaV7chSbm+K1gtYhiFB5qD2gxYym2r/e++UJG/hOeod3tUgrrObC9q S+o20S3jS31CLhyAMgdq4cxDhDhY9pkJNTWABfqD2/lTIt1azX7SVcc4Z3nomwSiP4 xR7VCcUXz6L7+afwLgpEXN/yiRb+RfcSHNo4j28ut8Y12IUuzm0ZroNBq3wxiAkYDC AwP8jA8hJoJpRRA57ViBJ70JPtiOlevU/aXB3P9I5yUGmsCC2s5bLkf/PeSw3e3BtS r5E//YLrO3g3BrpZcWqr248DT6xiFou8CqalF2z++GVIheQ8NorC8T1fqoK8X5u6Cu LzQv4JRTS6rnA== Message-ID: Date: Sat, 20 Dec 2025 19:01:32 +0100 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] powerpc/fsl_rio: fix a improper release in fsl_rio_setup() To: Haoxiang Li Cc: alex.bou9@gmail.com, linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, maddy@linux.ibm.com, mpe@ellerman.id.au, mporter@kernel.crashing.org, npiggin@gmail.com References: <20251220134230.712676-1-lihaoxiang@isrc.iscas.ac.cn> Content-Language: fr-FR From: "Christophe Leroy (CS GROUP)" In-Reply-To: <20251220134230.712676-1-lihaoxiang@isrc.iscas.ac.cn> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Le 20/12/2025 à 14:42, Haoxiang Li a écrit : > [Vous ne recevez pas souvent de courriers de lihaoxiang@isrc.iscas.ac.cn. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ] > > On Sat, 20 Dec 2025 11:45:10 +0100, Christophe Leroy wrote: >> Your explanation is unclear. It is correct that put_device() is the >> correct way to drop the device reference, and it is already what >> rio_register_mport() does. Why do you need to move it out of >> rio_register_mport() ? This is what you have to explain. > > Sorry for the unclear changelog. I think if device_register() in > rio_register_mport() fails, put_device() is already called. Then, > the error path in fsl_rio_setup() also kfree() the port. I suspected > a potential double-free issue here, although I am unsure whether this interpretation is correct. The put_device() in rio_register_mport() releases the reference to port->dev taken by device_register(). See the documentation in https://elixir.bootlin.com/linux/v6.18/source/drivers/base/core.c#L3750 The kfree(port) in fsl_rio_setup() is to free the memory allocated by the kzalloc() at https://elixir.bootlin.com/linux/v6.18/source/arch/powerpc/sysdev/fsl_rio.c#L583 Those are two independant things. > >> AI code review or human code review ? > > Human review. > >> Why do you remove this kfree() ? By doing this you will leak the port >> pointer allocated earlier in the loop by kzalloc() > > I just thought after device_register(), the cleanup should be done by > put_device(). You are talking about the cleanup of port->dev. Once you have cleaned up port->dev you also have to cleanup port. > > I would appreciate your clarification. See above. I might be wrong but from my point of view the existing code is correct. Christophe