linux-kernel - Re: [PATCH 22/24] irqchip/gic-v5: Add GICv5 ITS support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z/d8bP9GceSVpJAU@lpieralisi>
Date: Thu, 10 Apr 2025 10:08:12 +0200
From: Lorenzo Pieralisi <lpieralisi@...nel.org>
To: Thomas Gleixner <tglx@...utronix.de>
Cc: Marc Zyngier <maz@...nel.org>, Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Catalin Marinas <catalin.marinas@....com>,
	Will Deacon <will@...nel.org>,
	Sascha Bischoff <sascha.bischoff@....com>,
	Timothy Hayes <timothy.hayes@....com>,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	devicetree@...r.kernel.org
Subject: Re: [PATCH 22/24] irqchip/gic-v5: Add GICv5 ITS support

On Wed, Apr 09, 2025 at 08:57:02PM +0200, Thomas Gleixner wrote:
> On Tue, Apr 08 2025 at 12:50, Lorenzo Pieralisi wrote:
> > +	guard(mutex)(&its->dev_alloc_lock);
> > +
> > +	its_dev = gicv5_its_find_device(its, device_id);
> > +	if (IS_ERR(its_dev))
> > +		return PTR_ERR(its_dev);
> > +
> > +	ret = gicv5_its_alloc_eventid(its_dev, nr_irqs, &event_id_base);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = iommu_dma_prepare_msi(info->desc,
> > +				    its->its_trans_phys_base);
> > +	if (ret)
> > +		goto out_eventid;
> 
> As 0-day already complained, guard() and goto does not mix. the simple
> solution is to do
> 
> foo()
>         guard()
>         return __foo();
> 
> Then you can put the gotos into __foo()

Yes even though its usage in this function IIUC is technically fine,
there are other kernel functions where the guard(mutex) is at the
function entry with gotos that follow.

Regardless, bad idea to mix them, as documented, thanks.

Side note: the bug the bot caught is because I moved the guard(mutex)
lower in the function and incidentally got past gotos (and gcc did not
catch it, it has to go up to W=2 to barf).

Anyway - as I mentioned in the cover letter, the current IWB driver
design, (patch 23 - where the guard(mutex) bug is and more readl_poll_*
boilerplate :), sorry), is a bit of a question (should I move it to
DOMAIN_BUS_WIRED_TO_MSI with the fixed eventid caveat to solve somehow),
or is it OK "as-is", treating it basically as an ITS hardcoded plugin ?

Thank you very much for having a look.

Lorenzo