From: Nicolin Chen <nicolinc@nvidia.com>
To: Robin Murphy <robin.murphy@arm.com>
Cc: <will@kernel.org>, <joro@8bytes.org>,
<thunder.leizhen@huawei.com>, <jgg@ziepe.ca>,
<tglx@linutronix.de>, <john.garry@huawei.com>,
<jean-philippe@linaro.org>, <christophe.jaillet@wanadoo.fr>,
<linux-arm-kernel@lists.infradead.org>,
<iommu@lists.linux-foundation.org>,
<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] iommu/arm-smmu-v3: Align size in __arm_smmu_tlb_inv_range
Date: Thu, 14 Apr 2022 20:56:57 -0700 [thread overview]
Message-ID: <YljtCbvaAHX2lKkF@Asurada-Nvidia> (raw)
In-Reply-To: <13c91dfb-c540-ed8d-daa7-eab7207df221@arm.com>
On Thu, Apr 14, 2022 at 11:32:38AM +0100, Robin Murphy wrote:
> External email: Use caution opening links or attachments
>
>
> On 2022-04-13 21:19, Nicolin Chen wrote:
> > Hi Robin,
> >
> > On Wed, Apr 13, 2022 at 02:40:31PM +0100, Robin Murphy wrote:
> > > On 2022-04-13 05:17, Nicolin Chen wrote:
> > > > To calculate num_pages, the size should be aligned with
> > > > "page size", determined by the tg value. Otherwise, its
> > > > following "while (iova < end)" might become an infinite
> > > > loop if unaligned size is slightly greater than 1 << tg.
> > >
> > > Hmm, how does a non-page-aligned invalidation request get generated in
> > > the first place?
> >
> > I don't have the testing environment because it was a bug,
> > reported by a client who uses SVA feature on top of SMMU.
> >
> > But judging from the log, the non-page-aligned inv request
> > was coming from an likely incorrect end address, e.g.
> > { start = 0xff10000, end = 0xff20000 }
> > So the size turned out to be 0x10001, unaligned.
> >
> > I don't have a full call trace on hand right now to see if
> > upper callers are doing something wrong when calculate the
> > end address, though I've asked the owner to check.
> >
> > By looking at the call trace within arm_smmu_* functions:
> > __arm_smmu_tlb_inv_range
> > arm_smmu_tlb_inv_range_asid
> > arm_smmu_mm_invalidate_range
> > {from mm_notifier_* functions}
> >
> > There's no address alignment check. Although I do think we
> > should fix the source who passes down the non-page-aligned
> > parameter, the SMMU driver shouldn't silently dead loop if
> > a set of unaligned inputs are given, IMHO.
>
> Oh, sure, I'm not saying we definitely don't need to fix anything, I'd
> just like to get a better understanding of *what* we're fixing. I'd have
> (naively) expected the mm layer to give us page-aligned quantities even
> in the SVA notifier case, so if we've got a clear off-by-one somewhere
> in that path we should fix that before just blindly over-invalidating to
> paper over it;
I see. Yea, definitely should fix the source too. I've asked
the owner to track it down. I sent the change, thinking that
we could do it in parallel.
> if we still also want to be robust at the SMMU driver end
> just in case, something like "if (WARN_ON(num_pages == 0)) num_pages =
> 1;" might be more appropriate. However if it turns out that we *can*
> actually end up with unsanitised input from some userspace unmap
> interface getting this far, then a silent fixup is the best option, but
> if so I'd still like to confirm that we're rounding in the same
> direction as whoever touched the pagetables (since it can't have been us).
I see. I'll give an update once I have the debugging result.
Thanks!
Nic
next prev parent reply other threads:[~2022-04-15 3:57 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-13 4:17 [PATCH] iommu/arm-smmu-v3: Align size in __arm_smmu_tlb_inv_range Nicolin Chen
2022-04-13 13:40 ` Robin Murphy
2022-04-13 20:19 ` Nicolin Chen
2022-04-14 10:32 ` Robin Murphy
2022-04-15 3:56 ` Nicolin Chen [this message]
2022-04-16 2:03 ` Nicolin Chen
2022-04-19 20:02 ` Jason Gunthorpe
2022-04-19 20:05 ` Nicolin Chen
2022-04-19 20:15 ` Robin Murphy
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=YljtCbvaAHX2lKkF@Asurada-Nvidia \
--to=nicolinc@nvidia.com \
--cc=christophe.jaillet@wanadoo.fr \
--cc=iommu@lists.linux-foundation.org \
--cc=jean-philippe@linaro.org \
--cc=jgg@ziepe.ca \
--cc=john.garry@huawei.com \
--cc=joro@8bytes.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=robin.murphy@arm.com \
--cc=tglx@linutronix.de \
--cc=thunder.leizhen@huawei.com \
--cc=will@kernel.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).