All of lore.kernel.org
 help / color / mirror / Atom feed
From: Janne Grunau <j@jannau.net>
To: Dan Carpenter <dan.carpenter@linaro.org>
Cc: Hector Martin <marcan@marcan.st>,
	asahi@lists.linux.dev, iommu@lists.linux.dev
Subject: Re: [bug report] iommu/io-pgtable-dart: Add 4-level page table support
Date: Tue, 9 Sep 2025 13:02:09 +0200	[thread overview]
Message-ID: <20250909110209.GD89417@robin.jannau.net> (raw)
In-Reply-To: <aMACFlJjrZHs_Yf-@stanley.mountain>

On Tue, Sep 09, 2025 at 01:31:50PM +0300, Dan Carpenter wrote:
> Hello Hector Martin,
> 
> Commit 74a0e72f03ff ("iommu/io-pgtable-dart: Add 4-level page table
> support") from Aug 21, 2025 (linux-next), leads to the following
> (UNPUBLISHED) Smatch static checker warning:
> 
> drivers/iommu/io-pgtable-dart.c:183 dart_get_last() warn: array off by one? 'data->pgd[tbl]'
> drivers/iommu/io-pgtable-dart.c:252 dart_map_pages() warn: array off by one? 'data->pgd[tbl]'
> 
> drivers/iommu/io-pgtable-dart.c
>     174 static dart_iopte *dart_get_last(struct dart_io_pgtable *data, unsigned long iova)
>     175 {
>     176         dart_iopte pte, *ptep;
>     177         int level = data->levels;
>     178         int tbl = dart_get_index(data, iova, level);
>     179 
>     180         if (tbl > (1 << data->tbl_bits))
>                         ^
> It does look like this should be >=.

yes. There is a second occurance of this buggy check in
dart_map_pages().

>     181                 return NULL;
>     182 
> --> 183         ptep = data->pgd[tbl];
> 
> data->pgd[] has BIT(2) elements.  The data->tbl_bits value is set
> in dart_alloc_pgtable() and it has a check if (tbl_bits > max_tbl_bits)
> which ensures that it is not more than 2.
> 
> I don't think dart_get_index() can actually return more than 3 so
> maybe the check can just be removed?

It can't as long as iova is within the iommu's ias. After brief check I
haven't seen anything in iommu/iommu.c or iommu/apple-dart.c which
prevents drivers to call iommu_map() with a random iova resulting in
tbl == 4. So the checks needs to be fixed.

I'll send a patch fixing both occurances.

Thanks for the report

Janne

      reply	other threads:[~2025-09-09 11:02 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-09 10:31 [bug report] iommu/io-pgtable-dart: Add 4-level page table support Dan Carpenter
2025-09-09 11:02 ` Janne Grunau [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250909110209.GD89417@robin.jannau.net \
    --to=j@jannau.net \
    --cc=asahi@lists.linux.dev \
    --cc=dan.carpenter@linaro.org \
    --cc=iommu@lists.linux.dev \
    --cc=marcan@marcan.st \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.