From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.mindbit.ro (xs1.mindbit.ro [80.86.107.70]) (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 3312F257824 for ; Sun, 2 Aug 2026 15:12:56 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=80.86.107.70 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785683580; cv=none; b=TWhM6anuEFUBh+uZ376d+DwyybVDoTd3STNHzxRK1XtKJL/ogcpbG4QDcyGBXmDdHOQ8D3hdpDoV5Y62AuTL28bM0C3/Dc7nO8zoTnON1jJnliuNyMwA9QotMBbxkO8A6vfGfLKvKX3eAvV7TlX4EFw8Lf/bTuDyt7F4n3LAfVM= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785683580; c=relaxed/simple; bh=EjOdFcPWB4KK3Cgqz+XJ9U/Z1czuZGObowHcC/UqRoA=; h=Message-ID:Subject:From:To:Cc:Date:In-Reply-To:References: Content-Type:MIME-Version; b=BfupqQ1Wzmqb6U9+ru7LQZ9WCSqUKte9lBIzQFbVDD11fzG7sAYJrcN3xGsmhF4eNwxPP8a1dkkPieE3FjEsyhwWZojFKC/U6PJUJuTRZ8QYoFOe5ABnkqu6XwcKYzYj6cDMKfEYSQzw+DYFa7CDAnEmJMrbT4QbihE9DnVXvvU= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=rendec.net; spf=pass smtp.mailfrom=rendec.net; dkim=pass (2048-bit key) header.d=rendec.net header.i=@rendec.net header.b=tbubWk+9; arc=none smtp.client-ip=80.86.107.70 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=rendec.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=rendec.net Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=rendec.net header.i=@rendec.net header.b="tbubWk+9" Received: from dog.kanata.rendec.net (pool-174-112-193-187.cpe.net.cable.rogers.com [174.112.193.187]) by mail.mindbit.ro (Postfix) with ESMTPSA id ED6D9CC485; Sun, 2 Aug 2026 18:12:52 +0300 (EEST) DKIM-Filter: OpenDKIM Filter v2.11.0 mail.mindbit.ro ED6D9CC485 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rendec.net; s=default; t=1785683574; bh=xLwRfl4wzX6dxUIxEMA1YWSpw5v6mfzdcCXNFiov2MA=; h=Subject:From:To:Cc:Date:In-Reply-To:References:From; b=tbubWk+9ZhTkP+7kwrMRWCNX5GueyTQWwepHk4kMk5IUs0mODG7KzRSxewcICHZZ0 QfL0q8eS8viIoc9RHoQ9MM4jWqsluPyxcndfC1T/T0z7ilx70cpPSTC5PcYUnhjzD0 aaksbrceJSB1ylUZyuAzTWJcmfVNbCoeOH673oblPIAaDzBafaUyCnWD1NE+xced0D Zg39WzECjls/0LiCQc/byXVFRYYZVMlwuS/pEVSimwDpNRWuhY9Cy2N8duIJMVNNpI 7TOsDBHz//+ekKFLbNSCw39wCndgWH3ka6jwxOvDDhbu+HK/ImsvCgmy+IDAtYHLX5 BYIj5U7cngf0Q== Message-ID: <95f737894638361435fb6af600e3c156dcea20ac.camel@rendec.net> Subject: Re: [PATCH 04/16] irqchip/crossbar: fix allocation and init cleanup From: Radu Rendec To: Haofeng Li <920484857@qq.com>, tglx@kernel.org Cc: linux-kernel@vger.kernel.org, Haofeng Li <13266079573@163.com>, Haofeng Li , Jason Cooper , Marc Zyngier , Tony Lindgren Date: Sun, 02 Aug 2026 11:12:50 -0400 In-Reply-To: References: <20260714122351.3274006-1-lihaofeng@kylinos.cn> <20260714132453.3302672-1-920484857@qq.com> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.58.3 (3.58.3-1.fc43) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 On Tue, 2026-07-14 at 21:24 +0800, Haofeng Li wrote: > From: Haofeng Li >=20 > crossbar_domain_alloc() returns in the middle of a multi-IRQ > allocation without freeing IRQs and crossbar slots allocated by > earlier iterations. >=20 > If irq_domain_create_hierarchy() fails after crossbar_of_init(), the > global crossbar object and its maps and MMIO mapping are also leaked. >=20 > Roll back prior IRQs through crossbar_domain_free(), and release all > crossbar resources when domain creation fails. >=20 > Fixes: 783d31863fb8 ("irqchip: crossbar: Convert dra7 crossbar to stacked= domains") >=20 > Signed-off-by: Haofeng Li > --- > =C2=A0drivers/irqchip/irq-crossbar.c | 23 +++++++++++++++++++++-- > =C2=A01 file changed, 21 insertions(+), 2 deletions(-) >=20 > diff --git a/drivers/irqchip/irq-crossbar.c b/drivers/irqchip/irq-crossba= r.c > index 033b08782119..0ac5268aa8e4 100644 > --- a/drivers/irqchip/irq-crossbar.c > +++ b/drivers/irqchip/irq-crossbar.c > @@ -42,6 +42,9 @@ struct crossbar_device { > =C2=A0 > =C2=A0static struct crossbar_device *cb; > =C2=A0 > +static void crossbar_domain_free(struct irq_domain *domain, unsigned int= virq, > + unsigned int nr_irqs); > + > =C2=A0static void crossbar_writel(int irq_no, int cb_no) > =C2=A0{ > =C2=A0 writel(cb_no, cb->crossbar_base + cb->register_offsets[irq_no]); > @@ -127,11 +130,14 @@ static int crossbar_domain_alloc(struct irq_domain = *d, unsigned int virq, > =C2=A0 for (i =3D 0; i < nr_irqs; i++) { > =C2=A0 int err =3D allocate_gic_irq(d, virq + i, hwirq + i); > =C2=A0 > - if (err) > + if (err) { > + if (i) I wouldn't bother checking if i !=3D 0. This is a corner case, and it's handled well, both inside crossbar_domain_free() itself and in irq_domain_free_irqs_parent(). > + crossbar_domain_free(d, virq, i); > =C2=A0 return err; > + } > =C2=A0 > =C2=A0 irq_domain_set_hwirq_and_chip(d, virq + i, hwirq + i, > - =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 &crossbar_chip, NULL); > + =C2=A0=C2=A0=C2=A0=C2=A0 &crossbar_chip, NULL); Why change the indentation here? > =C2=A0 } > =C2=A0 > =C2=A0 return 0; > @@ -336,6 +342,18 @@ static int __init crossbar_of_init(struct device_nod= e *node) > =C2=A0 return ret; > =C2=A0} > =C2=A0 > +static void __init crossbar_free(void) > +{ > + if (!cb) > + return; cb cannot be NULL here. In fact, the only way this code can be reached is after the call to irq_domain_create_hierarchy() in irqcrossbar_init() which already dereferences cb. > + > + kfree(cb->register_offsets); > + kfree(cb->irq_map); > + iounmap(cb->crossbar_base); > + kfree(cb); > + cb =3D NULL; > +} > + > =C2=A0static int __init irqcrossbar_init(struct device_node *node, > =C2=A0 =C2=A0=C2=A0 struct device_node *parent) > =C2=A0{ > @@ -361,6 +379,7 @@ static int __init irqcrossbar_init(struct device_node= *node, > =C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0 of_fwnode_handle(node), &crossbar_dom= ain_ops, NULL); > =C2=A0 if (!domain) { > =C2=A0 pr_err("%pOF: failed to allocated domain\n", node); > + crossbar_free(); Why create a separate function and not do the cleanup here? It's not called anywhere else, and the driver cannot be compiled as a loadable module (and therefore doesn't need a global _free() function). > =C2=A0 return -ENOMEM; > =C2=A0 } > =C2=A0