Re: [PATCH] mtd: virt_concat: fix use-after-free in mtd_virt_concat_destroy_joins()

From: "Luca Ceresoli" <luca.ceresoli@bootlin.com>
To: "Dan Carpenter" <error27@gmail.com>,
	"Luca Ceresoli" <luca.ceresoli@bootlin.com>
Cc: "Harshit Mogalapalli" <harshit.m.mogalapalli@oracle.com>,
	"Miquel Raynal" <miquel.raynal@bootlin.com>,
	"Richard Weinberger" <richard@nod.at>,
	"Vignesh Raghavendra" <vigneshr@ti.com>,
	"Amit Kumar Mahapatra" <amit.kumar-mahapatra@amd.com>,
	<linux-mtd@lists.infradead.org>, <linux-kernel@vger.kernel.org>,
	<kernel-janitors@vger.kernel.org>
Subject: Re: [PATCH] mtd: virt_concat: fix use-after-free in mtd_virt_concat_destroy_joins()
Date: Tue, 16 Jun 2026 22:31:28 +0200	[thread overview]
Message-ID: <DJARC89F2VJR.1EDEQ27GSDAK6@bootlin.com> (raw)
In-Reply-To: <ajEhuS8xwcX1AA_I@stanley.mountain>

On Tue Jun 16, 2026 at 12:13 PM CEST, Dan Carpenter wrote:
> On Tue, Jun 16, 2026 at 11:51:53AM +0200, Luca Ceresoli wrote:
>> Issue 2: in mtd_virt_concat_destroy_joins():
>>
>> 	list_for_each_entry_safe(item, tmp, &concat_node_list, head) {
>> 		mtd = &item->concat->mtd; [0]
>> 		if (item->concat) { [1]
>>
>> At line [0] we dereference item->concat, but at line [1] we apparently
>> handle the case where item->concat can be NULL. Either [1] is always true
>> and we can remove the if(), or [1] can be false, so [0] is a bug and should
>> probably be moved to inside the if().
>
> That's not a dereference on line 0, it's pointer math.  So the code
> works.  But a lot of people find the distinction confusing.

Ah, indeed, you are right on both aspects: it's just pointer math (not a
bug) + it is confusing code.

So moving [0] inside the if() would be a readability improvement IMO, but
definitely not a bugfix.

Luca

--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com