Re: [PATCH] iommu/amd: Use array_index_nospec() for rlookup_table index

[Dan Carpenter <dan.carpenter@linaro.org>]

On Fri, Jan 09, 2026 at 02:19:03PM +0530, Dheeraj Kumar Srivastava wrote:
> Hi Dan,
> 
> On 1/9/2026 11:35 AM, Dan Carpenter wrote:
> > On Fri, Jan 09, 2026 at 10:50:40AM +0530, Dheeraj Kumar Srivastava wrote:
> > > Use array_index_nospec() to prevent speculative out-of-bounds
> > > access when indexing pci_seg->rlookup_table with a user provided
> > > device id.
> > > 
> > > Signed-off-by: Dheeraj Kumar Srivastava <dheerajkumar.srivastava@amd.com>
> > > Reviewed-by: Sairaj Kodilkar <Sairaj.ArunKodilkar@amd.com>
> > > Reported-by: kernel test robot <lkp@intel.com>
> > > Reported-by: Dan Carpenter <error27@gmail.com>
> > > Closes: https://lore.kernel.org/r/202510281233.q4cBnp3z-lkp@intel.com/
> > 
> > This is interesting because more and more people are using lei to
> > recieve email and now they get unfiltered Smatch warnings from zero day
> > bot.
> > 
> > Normally, I just ignore these warnings because they're hard to review
> > and I recently modified Smatch to stop the zero day bot from warning
> > about them.
> > 
> > The problem is that I've tried to contact people from Intel to help
> > review some of the warnings but I've never recieved a response.  I've
> > heard that Intel has a handful of people that deal with Spectre v1 bugs
> > but I've never seen any evidence of that...  I've never tried reaching
> > out to AMD.
> > 
> > > ---
> > >   drivers/iommu/amd/debugfs.c | 1 +
> > >   1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/drivers/iommu/amd/debugfs.c b/drivers/iommu/amd/debugfs.c
> > > index 10fa217a7119..4990f6db99ef 100644
> > > --- a/drivers/iommu/amd/debugfs.c
> > > +++ b/drivers/iommu/amd/debugfs.c
> > > @@ -174,6 +174,7 @@ static ssize_t devid_write(struct file *filp, const char __user *ubuf,
> > >   			kfree(srcid_ptr);
> > >   			return -EINVAL;
> > >   		}
> > > +		devid = array_index_nospec(devid, (u32)pci_seg->last_bdf + 1);
> > 
> > This is debugfs so it's already root only.  The cast to (u32) is
> > unnecessary.
> > 
> 
> I agree that the (u32) cast is unnecessary here and will remove it.
> 
> When you mentioned that this is debugfs and therefore root-only, could you
> clarify the context of that comment? I just want to make sure I’m
> interpreting the rationale correctly.
> 

Debugfs is always read only unless you are root.  Anything else is insane.
So it's like sure, this patch is correct from a correctness point of view
but in terms of is this exploitable?  No.

regards,
dan carpenter