linux-kernel - Re: [PATCH v14 08/10] iommu/arm-smmu-v3: Add in-kernel support for NVIDIA Tegra241 (Grace) CMDQV

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: <fcf44982-166a-4a25-acd0-02a142e205d7@stanley.mountain>
Date: Wed, 4 Sep 2024 20:12:19 +0300
From: Dan Carpenter <dan.carpenter@...aro.org>
To: Nicolin Chen <nicolinc@...dia.com>
Cc: will@...nel.org, robin.murphy@....com, joro@...tes.org, jgg@...dia.com,
	thierry.reding@...il.com, vdumpa@...dia.com, jonathanh@...dia.com,
	linux-kernel@...r.kernel.org, iommu@...ts.linux.dev,
	linux-arm-kernel@...ts.infradead.org, linux-tegra@...r.kernel.org
Subject: Re: [PATCH v14 08/10] iommu/arm-smmu-v3: Add in-kernel support for
 NVIDIA Tegra241 (Grace) CMDQV

On Wed, Sep 04, 2024 at 08:17:11AM -0700, Nicolin Chen wrote:
> Hi Dan,
> 
> On Wed, Sep 04, 2024 at 10:29:26AM +0300, Dan Carpenter wrote:
>  
> > I was reviewing Smatch warnings:
> > 
> >     drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c:616 tegra241_cmdqv_init_vintf()
> >     error: Calling ida_alloc_max() with a 'max' argument which is a power of 2. -1 missing?
> > 
> > The problem is that we're calling ida_alloc_max() where max is always zero.
> > 
> > > +static int tegra241_cmdqv_init_vintf(struct tegra241_cmdqv *cmdqv, u16 max_idx,
> > > +                                  struct tegra241_vintf *vintf)
> > > +{
> > > +
> > > +     u16 idx;
> > > +     int ret;
> > > +
> > > +     ret = ida_alloc_max(&cmdqv->vintf_ids, max_idx, GFP_KERNEL);
> > > +     if (ret < 0)
> > > +             return ret;
> > > +     idx = ret;
> > 
> > max_idx is always zero so idx is always zero.
> 
> There is a followup series adding support for max[1, max_vintf].
> And I guess that would make Smatch happy. I'd personally prefer
> keep this by ignoring the Smatch warning. But if you think the
> common practice is to drop it and add back, I'd be okay with it.
> 

I'm just reviewing static checker warnings so I don't know the back story...
How long are we going to have to wait for the follow on patchset?

Generally if someone had noticed this in review they would have asked that it
be dropped but now that it's in, you're probably in the clear.  No one else is
going to volunteer to refactor this code if you don't.  ;)

With regards, to ignoring static checker warnings.  These are one time emails.
There is always going to be a certain percent of false positives.  You're
*encouraged* to ignore static checker warnings unless it's a bug or it makes the
code cleaner.  The goal is never to silence the checker.

regards,
dan carpenter