All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nicolin Chen <nicolinc@nvidia.com>
To: Mostafa Saleh <smostafa@google.com>
Cc: Pranjal Shrivastava <praan@google.com>, <jgg@nvidia.com>,
	<will@kernel.org>, <joro@8bytes.org>, <robin.murphy@arm.com>,
	<linux-arm-kernel@lists.infradead.org>, <iommu@lists.linux.dev>,
	<linux-kernel@vger.kernel.org>, <linux-tegra@vger.kernel.org>
Subject: Re: [PATCH v3 2/2] iommu/arm-smmu-v3: Replace vsmmu_size/type with get_viommu_size
Date: Fri, 25 Jul 2025 12:01:39 -0700	[thread overview]
Message-ID: <aIPUkzn3ZdgbKRzG@Asurada-Nvidia> (raw)
In-Reply-To: <aIPI9xv-HxTPWMUp@google.com>

On Fri, Jul 25, 2025 at 06:12:07PM +0000, Mostafa Saleh wrote:
> On Fri, Jul 25, 2025 at 09:24:23AM -0700, Nicolin Chen wrote:
> > On Fri, Jul 25, 2025 at 09:18:35AM +0000, Mostafa Saleh wrote:
> > > > > > > On Wed, Jul 23, 2025 at 01:37:53PM +0000, Pranjal Shrivastava wrote:
> > > > > > > > On Mon, Jul 21, 2025 at 01:04:44PM -0700, Nicolin Chen wrote:
> > > > > Had the
> > > > > vintf_size rejected it, we wouldn't be calling the init op.
> > > > 
> > > > A data corruption could happen any time, not related to the
> > > > init op. A concurrent buggy thread can overwrite the vIOMMU
> > > > object when a write access to its adjacent memory overflows.
> > > 
> > > Can you please elaborate on that, as memory corruption can happen
> > > any time event after the next check and there is no way to defend
> > > against that?
> > 
> > That narrative is under a condition (in the context) "when there
> > is a kernel bug corrupting data" :)
> > 
> > E.g. some new lines of code allocates a wrong size of memory and
> > writes above the size. If that memory is near this vIOMMU object
> > it might overwrite to this vIOMMU object that this function gets.
> > 
> > This certainly won't happen if everything is sane.
> 
> I see, but I don't think we should do anything about that, there are
> 100s of structs in the kernel, we can't add checks everywhere, and I
> don't see anything special about this path to add an assertion, this
> kind of defensive programming isn't really helpful. We just need to
> review any new code properly :)

It could help for debugging purpose when writing new lines of code.
Kernel has quite a lot of WARN_ONs fencing something that shouldn't
happen.

With that being said, I admit that this particular line is a bit of
overreacting. Removing it doesn't have too big impact, as something
else would likely crash when such a corruption does happen.

Nicolin


      reply	other threads:[~2025-07-25 19:05 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-21 20:04 [PATCH v3 0/2] iommu/arm-smmu-v3: Two vsmmu impl_ops cleanups Nicolin Chen
2025-07-21 20:04 ` [PATCH v3 1/2] iommu/arm-smmu-v3: Do not bother impl_ops if IOMMU_VIOMMU_TYPE_ARM_SMMUV3 Nicolin Chen
2025-07-23 13:19   ` Pranjal Shrivastava
2025-07-21 20:04 ` [PATCH v3 2/2] iommu/arm-smmu-v3: Replace vsmmu_size/type with get_viommu_size Nicolin Chen
2025-07-23 13:37   ` Pranjal Shrivastava
2025-07-23 18:05     ` Nicolin Chen
2025-07-23 18:58       ` Pranjal Shrivastava
2025-07-24 20:55         ` Pranjal Shrivastava
2025-07-24 21:49           ` Nicolin Chen
2025-07-25  5:11             ` Pranjal Shrivastava
2025-07-25 16:03               ` Nicolin Chen
2025-07-25 17:47                 ` Pranjal Shrivastava
2025-07-25  9:18             ` Mostafa Saleh
2025-07-25 16:24               ` Nicolin Chen
2025-07-25 18:12                 ` Mostafa Saleh
2025-07-25 19:01                   ` Nicolin Chen [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=aIPUkzn3ZdgbKRzG@Asurada-Nvidia \
    --to=nicolinc@nvidia.com \
    --cc=iommu@lists.linux.dev \
    --cc=jgg@nvidia.com \
    --cc=joro@8bytes.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=praan@google.com \
    --cc=robin.murphy@arm.com \
    --cc=smostafa@google.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 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.