* Re: [PATCH 1/1] KVM: arm64: nv: Avoid full shadow s2 unmap
From: Wei-Lin Chang @ 2026-04-24 19:45 UTC (permalink / raw)
To: Marc Zyngier
Cc: linux-arm-kernel, kvmarm, linux-kernel, Oliver Upton, Joey Gouly,
Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Will Deacon
In-Reply-To: <867bq72n7l.wl-maz@kernel.org>
On Thu, Apr 16, 2026 at 11:50:38AM +0100, Marc Zyngier wrote:
> On Thu, 16 Apr 2026 00:05:40 +0100,
> Wei-Lin Chang <weilin.chang@arm.com> wrote:
> >
> > On Wed, Apr 15, 2026 at 09:38:55AM +0100, Marc Zyngier wrote:
>
> [...]
>
> > > > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> > > > index 851f6171751c..a97bd461c1e1 100644
> > > > --- a/arch/arm64/include/asm/kvm_host.h
> > > > +++ b/arch/arm64/include/asm/kvm_host.h
> > > > @@ -217,6 +217,10 @@ struct kvm_s2_mmu {
> > > > */
> > > > bool nested_stage2_enabled;
> > > >
> > > > + /* canonical IPA to nested IPA range lookup */
> > > > + struct maple_tree nested_revmap_mt;
> > > > + bool nested_revmap_broken;
> > > > +
> > >
> > > Consider moving this boolean next to the other ones so that you don't
> > > create too many holes in the kvm_s2_mmu structure (use pahole to find out).
> > >
> > > But I have some misgivings about the way things are structured
> > > here. Only NV needs a revmap, yet this is present irrelevant of the
> > > nature of the VM and bloats the data structure a bit.
> > >
> > > My naive approach would have been to only keep a pointer to the
> > > revmap, and make that pointer NULL when the tree is "broken", and
> > > freed under RCU if the context isn't the correct one.
> >
> > Can you explain what you mean by "if the context isn't the correct one"?
> > If this refers to when selecting a specific kvm_s2_mmu instance for
> > another context, then IIUC refcnt would already be 0 and there would be
> > no other user of the tree.
>
> Sorry, "context" is an overloaded word. I meant a situation in which
> you couldn't immediately free the maple-tree because you're holding
> locks and freeing (hypothetically) requires a sleeping "context". in
> this case, freeing under RCU, purely as a deferring mechanism, might
> be useful.
I experimented using RCU to free the tree as a deferring mechanism.
Here are a few observations:
- At reverse map record time, if maple tree store fails, we have to
change the maple tree pointer to a NULL, which is an RCU write
operation. Therefore we need to either take another lock, or use a
xchg(ptr, NULL) to avoid the lock.
- Because we're holding the read-side mmu_lock, we shouldn't block
during reverse map record. Therefore we should use call_rcu()
instead of synchronize_rcu() to free the "broken" tree. This implies
a pointer to a maple tree in kvm_s2_mmu will not suffice, an
additional structure with both the maple tree and an rcu_head have
to be created.
IMO looking at RCU calls mixed with mtree_{, un}lock(), and having a new
wrapper struct to make this dynamic allocation scheme work is not very
attractive to me.
Instead, what do you think if I aggregate all strictly NV-related
fields in kvm_s2_mmu i.e. tlb_vttbr, tlb_vtcr, nested_stage2_enabled,
shadow_pt_debugfs_dentry, pending_unmap, into a struct maybe called
kvm_s2_mmu_nested, add a maple tree in it, and have a pointer to this
struct in kvm_s2_mmu? kvm_s2_mmu_nested can then be allocated only if we
init a nested s2 mmu.
Do you think this can work and is better than the current approaches?
Thanks,
Wei-Lin Chang
>
> [...]
>
> > > > +/*
> > > > + * Per shadow S2 reverse map (IPA -> nested IPA range) maple tree payload
> > > > + * layout:
> > > > + *
> > > > + * bit 63: valid, 1 for non-polluted entries, prevents the case where the
> > > > + * nested IPA is 0 and turns the whole value to 0
> > > > + * bits 55-12: nested IPA bits 55-12
> > > > + * bit 0: polluted, 1 for polluted, 0 for not
> > > > + */
> > > > +#define VALID_ENTRY BIT(63)
> > > > +#define NESTED_IPA_MASK GENMASK_ULL(55, 12)
> > > > +#define UNKNOWN_IPA BIT(0)
> > > > +
> > >
> > > This only works because you are using the "advanced" API, right?
> > > Otherwise, you'd be losing the high bit. It'd be good to add a comment
> > > so that people keep that in mind.
> >
> > Sorry, I can't find any relationship between the advanced API and the
> > top most bit of the maple tree value, what am I missing?
>
> From Documentation/core-api/maple_tree.rst:
>
> <quote>
> The Maple Tree can store values between ``0`` and ``ULONG_MAX``. The Maple
> Tree reserves values with the bottom two bits set to '10' which are below 4096
> (ie 2, 6, 10 .. 4094) for internal use. If the entries may use reserved
> entries then the users can convert the entries using xa_mk_value() and convert
> them back by calling xa_to_value(). If the user needs to use a reserved
> value, then the user can convert the value when using the
> :ref:`maple-tree-advanced-api`, but are blocked by the normal API.
> </quote>
>
> So depending how you read this, you can conclude that the bit patterns
> you encode in the MT may be considered as invalid. xa_mk_value() would
> make things always work, but that shifts the value left by one bit,
> hence you'd lose bit 63 (see how we use trap_config in
> emulate-nested.c to deal with this).
>
> I think you are lucky that bits [11:1] are always 0 here, but that
> looks extremely fragile to me, so you never hit the [1:0]==10
> condition, but that's really fragile.
>
> >
> > >
> > > > void kvm_init_nested(struct kvm *kvm)
> > > > {
> > > > kvm->arch.nested_mmus = NULL;
> > > > @@ -769,12 +783,57 @@ static struct kvm_s2_mmu *get_s2_mmu_nested(struct kvm_vcpu *vcpu)
> > > > return s2_mmu;
> > > > }
> > > >
> > > > +void kvm_record_nested_revmap(gpa_t ipa, struct kvm_s2_mmu *mmu,
> > > > + gpa_t fault_ipa, size_t map_size)
> > > > +{
> > > > + struct maple_tree *mt = &mmu->nested_revmap_mt;
> > > > + gpa_t start = ipa;
> > > > + gpa_t end = ipa + map_size - 1;
> > > > + u64 entry, new_entry = 0;
> > > > + MA_STATE(mas, mt, start, end);
> > > > +
> > > > + if (mmu->nested_revmap_broken)
> > > > + return;
> > > > +
> > > > + mtree_lock(mt);
> > > > + entry = (u64)mas_find_range(&mas, end);
> > > > +
> > > > + if (entry) {
> > > > + /* maybe just a perm update... */
> > > > + if (!(entry & UNKNOWN_IPA) && mas.index == start &&
> > > > + mas.last == end &&
> > > > + fault_ipa == (entry & NESTED_IPA_MASK))
> > > > + goto unlock;
> > > > + /*
> > > > + * Create a "polluted" range that spans all the overlapping
> > > > + * ranges and store it.
> > > > + */
> > > > + while (entry && mas.index <= end) {
> > > > + start = min(mas.index, start);
> > > > + end = max(mas.last, end);
> > > > + entry = (u64)mas_find_range(&mas, end);
> > > > + }
> > > > + new_entry |= UNKNOWN_IPA;
> > > > + } else {
> > > > + new_entry |= fault_ipa;
> > > > + new_entry |= VALID_ENTRY;
> > > > + }
> > > > +
> > > > + mas_set_range(&mas, start, end);
> > > > + if (mas_store_gfp(&mas, (void *)new_entry, GFP_NOWAIT | __GFP_ACCOUNT))
> > > > + mmu->nested_revmap_broken = true;
> > >
> > > Can we try and minimise the risk of allocation failure here?
> > >
> > > user_mem_abort() tries very hard to pre-allocate pages for page
> > > tables by maintaining an memcache. Can we have a similar approach for
> > > the revmap?
> >
> > Unfortunately, as I understand the maple tree can only pre-allocate for
> > a store when the range and the entry to be stored is given, but in this
> > case we must inspect the tree to get that information after we hold the
> > mmu and maple tree locks. It is possible to do a two pass approach:
> >
> > pre-allocate -> take MMU lock -> take maple tree lock -> revalidate what
> > we pre-allocated is still usable (nobody changed the tree before we took
> > the maple tree lock)
> >
> > But I am not fond of this extra complexity..
>
> Fair enough. It would at least be interesting to get a feel for how
> often this happens, because if we fail often, it won't help much.
>
> [...]
>
> > > My other concern here is related to TLB invalidation. As the guest
> > > performs TLB invalidations that remove entries from the shadow S2,
> > > there is no way to update the revmap to account for this.
> > >
> > > This obviously means that the revmap becomes more and more inaccurate
> > > over time, and that is likely to accumulate conflicting entries.
> > >
> > > What is the plan to improve the situation on this front?
> >
> > Right now I think using a direct map which goes from nested IPA to
> > canonical IPA could work while not generating too much complexity, if we
> > keep the reverse map and direct map in lockstep (direct map keeping the
> > same mappings as the reverse map but just in reverse).
>
> Right, so that'd effectively a mirror of the guest's page tables at
> the point of taking the fault.
>
> > I'll try to do that and include it in the next iteration.
>
> Thanks,
>
> M.
>
> --
> Without deviation from the norm, progress is not possible.
^ permalink raw reply
* Re: [RFC PATCH v2 1/4] security: ima: call ima_init() again at late_initcall_sync for defered TPM
From: Paul Moore @ 2026-04-24 20:15 UTC (permalink / raw)
To: Yeoreum Yun, Mimi Zohar, roberto.sassu
Cc: Jonathan McDowell, linux-security-module, linux-kernel,
linux-integrity, linux-arm-kernel, kvmarm, jmorris, serge,
dmitry.kasatkin, eric.snowberg, jarkko, jgg, sudeep.holla, maz,
oupton, joey.gouly, suzuki.poulose, yuzenghui, catalin.marinas,
will, noodles, sebastianene
In-Reply-To: <aesGU8a3mbVzvteH@e129823.arm.com>
On Fri, Apr 24, 2026 at 1:57 AM Yeoreum Yun <yeoreum.yun@arm.com> wrote:
> > On Thu, Apr 23, 2026 at 2:13 PM Yeoreum Yun <yeoreum.yun@arm.com> wrote:
> > >
> > > Sounds good. Once the patch is posted, I’ll review it as well.
> > > Sorry again for the noise, and thanks for your patience ;)
> >
> > My apologies for not getting a chance to look at this patchset sooner.
> >
> > This seems like an obvious, perhaps even stupid, question, but I have
> > to ask: if IMA can be properly initialized via late_initcall_sync(),
> > why not simply do the initialization in late_initcall_sync() and drop
> > the late_initcall() initialization?
> >
> > Does any IMA functionality suffer if initialization waits until
> > late_initcall_sync()? If so, it seems non-critical if waiting until
> > _sync() is acceptable, as it appears in these patches/comments.
>
> This is the way first patch did, and here is some discussion for this
> (Might you have seen, but in case of you missed):
> - https://lore.kernel.org/all/a6a0e15286c983d720de227c6827adbe976c5b9b.camel@linux.ibm.com/
Thanks for the pointer.
Unfortunately, my concern remains the same: it's either "safe" to
initialize IMA at late_initcall_sync() or it isn't. Attempting to
initialize IMA twice seems both odd and wrong.
I understand the need to ensure that the TPM is available, but if it
isn't safe to wait to initialize IMA at late_initcall_sync() then it
would seem like this is a bad option and we need another mechanism to
synchronize IMA with TPM devices. If it is safe to initalize IMA in
late_initcall_sync(), just do that and be done with it.
I'm also guessing a two stage init process, e.g. some in
late_initcall() and some in late_initcall_sync(), doesn't make much
sense here, but that could be one other thing to consider if some IMA
tasks must be done in late_initcall().
--
paul-moore.com
^ permalink raw reply
* Re: [RFC PATCH v3 2/4] security: ima: call ima_init() again at late_initcall_sync for defered TPM
From: Mimi Zohar @ 2026-04-24 20:25 UTC (permalink / raw)
To: Jonathan McDowell, linux-security-module, linux-kernel,
linux-integrity, linux-arm-kernel, kvmarm
Cc: paul, jmorris, serge, roberto.sassu, dmitry.kasatkin,
eric.snowberg, jarkko, jgg, sudeep.holla, maz, oupton, joey.gouly,
suzuki.poulose, yuzenghui, catalin.marinas, will, noodles,
sebastianene, Yeoreum Yun
In-Reply-To: <5552c20c6d6d2ae3bbb6b35124af5d98d2f79163.1777036497.git.noodles@meta.com>
Thanks, Jonathan!
On Fri, 2026-04-24 at 14:24 +0100, Jonathan McDowell wrote:
> -static int __init init_ima(void)
> +static int __init init_ima(bool late)
> {
> int error;
>
> @@ -1247,10 +1247,26 @@ static int __init init_ima(void)
> return 0;
> }
>
> + /*
> + * If we found the TPM during our first attempt, or we know there's no
> + * TPM, nothing further to do
> + */
Perhaps it's just me, but the comment wording is a bit off. Could I change it
to: If we either found the TPM or knew there's no TPM during our first attempt,
nothing futher to do.
Otherwise the patch looks good.
Mimi
> + if (late && (ima_tpm_chip || !IS_ENABLED(CONFIG_TCG_TPM)))
> + return 0;
> +
> + ima_tpm_chip = tpm_default_chip();
> + if (!ima_tpm_chip && !late && IS_ENABLED(CONFIG_TCG_TPM)) {
> + pr_debug("TPM not available, will try later\n");
> + return -EPROBE_DEFER;
> + }
> +
> + if (!ima_tpm_chip)
> + pr_info("No TPM chip found, activating TPM-bypass!\n");
> +
^ permalink raw reply
* Re: [PATCH rc v2 1/5] iommu/arm-smmu-v3: Add arm_smmu_adopt_strtab() for kdump
From: Jason Gunthorpe @ 2026-04-24 20:50 UTC (permalink / raw)
To: Nicolin Chen
Cc: will, robin.murphy, kevin.tian, joro, praan, baolu.lu,
miko.lenczewski, smostafa, linux-arm-kernel, iommu, linux-kernel,
stable, jamien
In-Reply-To: <aeu3bNxCsy8azLOO@Asurada-Nvidia>
On Fri, Apr 24, 2026 at 11:33:16AM -0700, Nicolin Chen wrote:
> On Fri, Apr 24, 2026 at 01:56:13PM -0300, Jason Gunthorpe wrote:
> > On Wed, Apr 15, 2026 at 02:17:36PM -0700, Nicolin Chen wrote:
> > > +static int arm_smmu_adopt_strtab_2lvl(struct arm_smmu_device *smmu, u32 cfg_reg,
> [..]
> > > + cfg->l2.l1tab = devm_memremap(
> > > + smmu->dev, dma, num_l1_ents * sizeof(struct arm_smmu_strtab_l1),
> > > + MEMREMAP_WB);
> >
> > WB shouldn't be unconditional? If the SMMU is working non-coherently
> > we need to map it NC. Same remark everwhere
>
> Hmm, I am trying to add a coherent-only gate for the series.
OK, may just add a comment to that effect here
> MEMREMAP_WC might work. But we cannot verify that on a coherent
> SMMU, right?
At most you could fake the smmu to noncoherent and check it maps the
right thing and assume the arch code does it right
Jason
^ permalink raw reply
* Re: [PATCH rc v2 2/5] iommu/arm-smmu-v3: Implement is_attach_deferred() for kdump
From: Jason Gunthorpe @ 2026-04-24 20:51 UTC (permalink / raw)
To: Nicolin Chen
Cc: will, robin.murphy, kevin.tian, joro, praan, baolu.lu,
miko.lenczewski, smostafa, linux-arm-kernel, iommu, linux-kernel,
stable, jamien
In-Reply-To: <aeu5/HsLwfhNWpbm@Asurada-Nvidia>
On Fri, Apr 24, 2026 at 11:44:12AM -0700, Nicolin Chen wrote:
> On Fri, Apr 24, 2026 at 01:59:27PM -0300, Jason Gunthorpe wrote:
> > On Wed, Apr 15, 2026 at 02:17:37PM -0700, Nicolin Chen wrote:
> > > +static bool arm_smmu_is_attach_deferred(struct device *dev)
> > > +{
> > > + struct arm_smmu_master *master = dev_iommu_priv_get(dev);
> > > + struct arm_smmu_device *smmu = master->smmu;
> > > + int i;
> > > +
> > > + if (!(smmu->options & ARM_SMMU_OPT_KDUMP))
> > > + return false;
> > > +
> > > + for (i = 0; i < master->num_streams; i++) {
> > > + u32 sid = master->streams[i].id;
> > > + struct arm_smmu_ste *step;
> > > +
> > > + /* Guard against unpopulated L2 entries in the adopted table */
> > > + if ((smmu->features & ARM_SMMU_FEAT_2_LVL_STRTAB) &&
> > > + !smmu->strtab_cfg.l2.l2ptrs[arm_smmu_strtab_l1_idx(sid)])
> > > + continue;
> >
> > This can probably just call arm_smmu_init_sid_strtab()
> >
> > I think it is OK to allocate another level 2 here and it also has
> > protections for SID out of range..
>
> Actually, sashiko pointed out that this guard is a dead code.
>
> arm_smmu_init_sid_strtab() is called in arm_smmu_insert_master().
Even better
Jason
^ permalink raw reply
* Re: [RFC PATCH v2 1/4] security: ima: call ima_init() again at late_initcall_sync for defered TPM
From: Mimi Zohar @ 2026-04-24 20:57 UTC (permalink / raw)
To: Paul Moore, Yeoreum Yun, roberto.sassu
Cc: Jonathan McDowell, linux-security-module, linux-kernel,
linux-integrity, linux-arm-kernel, kvmarm, jmorris, serge,
dmitry.kasatkin, eric.snowberg, jarkko, jgg, sudeep.holla, maz,
oupton, joey.gouly, suzuki.poulose, yuzenghui, catalin.marinas,
will, noodles, sebastianene
In-Reply-To: <CAHC9VhSaT_quKYnpFjAfqvL07JNbWMgM6c4pB9F46NHawX3DCA@mail.gmail.com>
On Fri, 2026-04-24 at 16:15 -0400, Paul Moore wrote:
> On Fri, Apr 24, 2026 at 1:57 AM Yeoreum Yun <yeoreum.yun@arm.com> wrote:
> > > On Thu, Apr 23, 2026 at 2:13 PM Yeoreum Yun <yeoreum.yun@arm.com> wrote:
> > > >
> > > > Sounds good. Once the patch is posted, I’ll review it as well.
> > > > Sorry again for the noise, and thanks for your patience ;)
> > >
> > > My apologies for not getting a chance to look at this patchset sooner.
> > >
> > > This seems like an obvious, perhaps even stupid, question, but I have
> > > to ask: if IMA can be properly initialized via late_initcall_sync(),
> > > why not simply do the initialization in late_initcall_sync() and drop
> > > the late_initcall() initialization?
> > >
> > > Does any IMA functionality suffer if initialization waits until
> > > late_initcall_sync()? If so, it seems non-critical if waiting until
> > > _sync() is acceptable, as it appears in these patches/comments.
> >
> > This is the way first patch did, and here is some discussion for this
> > (Might you have seen, but in case of you missed):
> > - https://lore.kernel.org/all/a6a0e15286c983d720de227c6827adbe976c5b9b.camel@linux.ibm.com/
>
> Thanks for the pointer.
>
> Unfortunately, my concern remains the same: it's either "safe" to
> initialize IMA at late_initcall_sync() or it isn't. Attempting to
> initialize IMA twice seems both odd and wrong.
Agreed. However, IMA should be initialized as soon as the TPM becomes
available, not delayed.
In patch 2/4 patch description, Jonathan describes the reasoning:
Unfortunately some TPM drivers (such as Arm FF-A, or SPI attached TPM
devices) are not reliably available during the initcall_late stage,
resulting in a log error:
ima: No TPM chip found, activating TPM-bypass!
and no measurements into the TPM by IMA. We can avoid this by doing IMA
init in the initcall_late_sync stage, after the drivers have completed
their init + registration.
Rather than do this everywhere, and needlessly delay the initialisation
of IMA when there is no need to do so, we continue to try to initialise
at the earlier stage, only deferring to the later point if the TPM is
not available yet.
>
> I understand the need to ensure that the TPM is available, but if it
> isn't safe to wait to initialize IMA at late_initcall_sync() then it
> would seem like this is a bad option and we need another mechanism to
> synchronize IMA with TPM devices. If it is safe to initalize IMA in
> late_initcall_sync(), just do that and be done with it.
Within the same initcall level there is no way of ordering the initialization.
Yeorum attempted to address the ordering issue in commit 0e0546eabcd6
("firmware: arm_ffa: Change initcall level of ffa_init() to rootfs_initcall"),
which is being reverted in this patch set.
Ordering within an initcall level needs to be fixed, but for now retrying at
late_initcall_sync works for some, hopefully most, cases.
>
> I'm also guessing a two stage init process, e.g. some in
> late_initcall() and some in late_initcall_sync(), doesn't make much
> sense here, but that could be one other thing to consider if some IMA
> tasks must be done in late_initcall().
Perhaps something to think about for the future.
Mimi
^ permalink raw reply
* [PATCH] ASoC: dt-bindings: mediatek,mt6351: convert to DT schema
From: Manish Baing @ 2026-04-24 21:03 UTC (permalink / raw)
To: lgirdwood, broonie, robh, krzk+dt, conor+dt
Cc: matthias.bgg, angelogioacchino.delregno, kaichieh.chuang,
linux-sound, devicetree, linux-kernel, linux-arm-kernel,
linux-mediatek, manishbaing2789
Convert MediaTek MT6351 Audio CODEC bindings from text format to
YAML schema to enable dtbs_check validation.
Signed-off-by: Manish Baing <manishbaing2789@gmail.com>
---
.../bindings/sound/mediatek,mt6351-sound.yaml | 32 +++++++++++++++++++
.../devicetree/bindings/sound/mt6351.txt | 16 ----------
2 files changed, 32 insertions(+), 16 deletions(-)
create mode 100644 Documentation/devicetree/bindings/sound/mediatek,mt6351-sound.yaml
delete mode 100644 Documentation/devicetree/bindings/sound/mt6351.txt
diff --git a/Documentation/devicetree/bindings/sound/mediatek,mt6351-sound.yaml b/Documentation/devicetree/bindings/sound/mediatek,mt6351-sound.yaml
new file mode 100644
index 000000000000..f9fab8962325
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/mediatek,mt6351-sound.yaml
@@ -0,0 +1,32 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/mediatek,mt6351-sound.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MediaTek MT6351 Audio CODEC
+
+maintainers:
+ - KaiChieh Chuang <kaichieh.chuang@mediatek.com>
+
+description:
+ MT6351 Audio CODEC is a part of the MediaTek MT6351 PMIC.
+ It communicates with the SoC through the MediaTek PMIC wrapper(pwrap).
+
+allOf:
+ - $ref: dai-common.yaml#
+
+properties:
+ compatible:
+ const: mediatek,mt6351-sound
+
+required:
+ - compatible
+
+additionalProperties: false
+
+examples:
+ - |
+ sound {
+ compatible = "mediatek,mt6351-sound";
+ };
diff --git a/Documentation/devicetree/bindings/sound/mt6351.txt b/Documentation/devicetree/bindings/sound/mt6351.txt
deleted file mode 100644
index 7fb2cb99245e..000000000000
--- a/Documentation/devicetree/bindings/sound/mt6351.txt
+++ /dev/null
@@ -1,16 +0,0 @@
-Mediatek MT6351 Audio Codec
-
-The communication between MT6351 and SoC is through Mediatek PMIC wrapper.
-For more detail, please visit Mediatek PMIC wrapper documentation.
-
-Must be a child node of PMIC wrapper.
-
-Required properties:
-
-- compatible : "mediatek,mt6351-sound".
-
-Example:
-
-mt6351_snd {
- compatible = "mediatek,mt6351-sound";
-};
--
2.43.0
^ permalink raw reply related
* Re: [RFC PATCH v2 1/4] security: ima: call ima_init() again at late_initcall_sync for defered TPM
From: Paul Moore @ 2026-04-24 21:08 UTC (permalink / raw)
To: Mimi Zohar
Cc: Yeoreum Yun, roberto.sassu, Jonathan McDowell,
linux-security-module, linux-kernel, linux-integrity,
linux-arm-kernel, kvmarm, jmorris, serge, dmitry.kasatkin,
eric.snowberg, jarkko, jgg, sudeep.holla, maz, oupton, joey.gouly,
suzuki.poulose, yuzenghui, catalin.marinas, will, noodles,
sebastianene
In-Reply-To: <014cf39aa8d6a0bcfa1a95c069675977ac67b843.camel@linux.ibm.com>
On Fri, Apr 24, 2026 at 4:58 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> On Fri, 2026-04-24 at 16:15 -0400, Paul Moore wrote:
> > On Fri, Apr 24, 2026 at 1:57 AM Yeoreum Yun <yeoreum.yun@arm.com> wrote:
> > > > On Thu, Apr 23, 2026 at 2:13 PM Yeoreum Yun <yeoreum.yun@arm.com> wrote:
> > > > >
> > > > > Sounds good. Once the patch is posted, I’ll review it as well.
> > > > > Sorry again for the noise, and thanks for your patience ;)
> > > >
> > > > My apologies for not getting a chance to look at this patchset sooner.
> > > >
> > > > This seems like an obvious, perhaps even stupid, question, but I have
> > > > to ask: if IMA can be properly initialized via late_initcall_sync(),
> > > > why not simply do the initialization in late_initcall_sync() and drop
> > > > the late_initcall() initialization?
> > > >
> > > > Does any IMA functionality suffer if initialization waits until
> > > > late_initcall_sync()? If so, it seems non-critical if waiting until
> > > > _sync() is acceptable, as it appears in these patches/comments.
> > >
> > > This is the way first patch did, and here is some discussion for this
> > > (Might you have seen, but in case of you missed):
> > > - https://lore.kernel.org/all/a6a0e15286c983d720de227c6827adbe976c5b9b.camel@linux.ibm.com/
> >
> > Thanks for the pointer.
> >
> > Unfortunately, my concern remains the same: it's either "safe" to
> > initialize IMA at late_initcall_sync() or it isn't. Attempting to
> > initialize IMA twice seems both odd and wrong.
>
> Agreed. However, IMA should be initialized as soon as the TPM becomes
> available, not delayed.
>
> In patch 2/4 patch description, Jonathan describes the reasoning:
>
> Unfortunately some TPM drivers (such as Arm FF-A, or SPI attached TPM
> devices) are not reliably available during the initcall_late stage,
> resulting in a log error:
>
> ima: No TPM chip found, activating TPM-bypass!
>
> and no measurements into the TPM by IMA. We can avoid this by doing IMA
> init in the initcall_late_sync stage, after the drivers have completed
> their init + registration.
>
> Rather than do this everywhere, and needlessly delay the initialisation
> of IMA when there is no need to do so, we continue to try to initialise
> at the earlier stage, only deferring to the later point if the TPM is
> not available yet.
Once again, that heavily implies that it is safe to initialize IMA in late-sync.
Put another way, what breaks if IMA's initialization is delayed to
late-sync? If the answer is nothing, just move the initialization to
late-sync. However, if something *is* broken and we are just doing
this because of TPM delays at boot, this patchset just creates
additional problems and we need a different solution. I can't
envision a scenario where it makes sense to attempt initialization
twice.
> > I understand the need to ensure that the TPM is available, but if it
> > isn't safe to wait to initialize IMA at late_initcall_sync() then it
> > would seem like this is a bad option and we need another mechanism to
> > synchronize IMA with TPM devices. If it is safe to initalize IMA in
> > late_initcall_sync(), just do that and be done with it.
>
> Within the same initcall level there is no way of ordering the initialization.
> Yeorum attempted to address the ordering issue in commit 0e0546eabcd6
> ("firmware: arm_ffa: Change initcall level of ffa_init() to rootfs_initcall"),
> which is being reverted in this patch set.
>
> Ordering within an initcall level needs to be fixed, but for now retrying at
> late_initcall_sync works for some, hopefully most, cases.
That's not a good answer. Ignoring the TPM issue for a moment, can
you confirm that moving IMA's initialization to late-sync is safe? If
not, why is this approach being considered?
--
paul-moore.com
^ permalink raw reply
* Re: [RFC PATCH v2 1/4] security: ima: call ima_init() again at late_initcall_sync for defered TPM
From: Mimi Zohar @ 2026-04-24 22:00 UTC (permalink / raw)
To: Paul Moore
Cc: Yeoreum Yun, roberto.sassu, Jonathan McDowell,
linux-security-module, linux-kernel, linux-integrity,
linux-arm-kernel, kvmarm, jmorris, serge, dmitry.kasatkin,
eric.snowberg, jarkko, jgg, sudeep.holla, maz, oupton, joey.gouly,
suzuki.poulose, yuzenghui, catalin.marinas, will, noodles,
sebastianene
In-Reply-To: <CAHC9VhTW3=RJ8L91RWXYYA9tFjfSXGN-DMEEwdiD6big9H57Ew@mail.gmail.com>
On Fri, 2026-04-24 at 17:08 -0400, Paul Moore wrote:
> On Fri, Apr 24, 2026 at 4:58 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> > On Fri, 2026-04-24 at 16:15 -0400, Paul Moore wrote:
> > > On Fri, Apr 24, 2026 at 1:57 AM Yeoreum Yun <yeoreum.yun@arm.com> wrote:
> > > > > On Thu, Apr 23, 2026 at 2:13 PM Yeoreum Yun <yeoreum.yun@arm.com> wrote:
> > > > > >
> > > > > > Sounds good. Once the patch is posted, I’ll review it as well.
> > > > > > Sorry again for the noise, and thanks for your patience ;)
> > > > >
> > > > > My apologies for not getting a chance to look at this patchset sooner.
> > > > >
> > > > > This seems like an obvious, perhaps even stupid, question, but I have
> > > > > to ask: if IMA can be properly initialized via late_initcall_sync(),
> > > > > why not simply do the initialization in late_initcall_sync() and drop
> > > > > the late_initcall() initialization?
> > > > >
> > > > > Does any IMA functionality suffer if initialization waits until
> > > > > late_initcall_sync()? If so, it seems non-critical if waiting until
> > > > > _sync() is acceptable, as it appears in these patches/comments.
> > > >
> > > > This is the way first patch did, and here is some discussion for this
> > > > (Might you have seen, but in case of you missed):
> > > > - https://lore.kernel.org/all/a6a0e15286c983d720de227c6827adbe976c5b9b.camel@linux.ibm.com/
> > >
> > > Thanks for the pointer.
> > >
> > > Unfortunately, my concern remains the same: it's either "safe" to
> > > initialize IMA at late_initcall_sync() or it isn't. Attempting to
> > > initialize IMA twice seems both odd and wrong.
> >
> > Agreed. However, IMA should be initialized as soon as the TPM becomes
> > available, not delayed.
> >
> > In patch 2/4 patch description, Jonathan describes the reasoning:
> >
> > Unfortunately some TPM drivers (such as Arm FF-A, or SPI attached TPM
> > devices) are not reliably available during the initcall_late stage,
> > resulting in a log error:
> >
> > ima: No TPM chip found, activating TPM-bypass!
> >
> > and no measurements into the TPM by IMA. We can avoid this by doing IMA
> > init in the initcall_late_sync stage, after the drivers have completed
> > their init + registration.
> >
> > Rather than do this everywhere, and needlessly delay the initialisation
> > of IMA when there is no need to do so, we continue to try to initialise
> > at the earlier stage, only deferring to the later point if the TPM is
> > not available yet.
>
> Once again, that heavily implies that it is safe to initialize IMA in late-sync.
>
> Put another way, what breaks if IMA's initialization is delayed to
> late-sync? If the answer is nothing, just move the initialization to
> late-sync. However, if something *is* broken and we are just doing
> this because of TPM delays at boot, this patchset just creates
> additional problems and we need a different solution. I can't
> envision a scenario where it makes sense to attempt initialization
> twice.
>
> > > I understand the need to ensure that the TPM is available, but if it
> > > isn't safe to wait to initialize IMA at late_initcall_sync() then it
> > > would seem like this is a bad option and we need another mechanism to
> > > synchronize IMA with TPM devices. If it is safe to initalize IMA in
> > > late_initcall_sync(), just do that and be done with it.
> >
> > Within the same initcall level there is no way of ordering the initialization.
> > Yeorum attempted to address the ordering issue in commit 0e0546eabcd6
> > ("firmware: arm_ffa: Change initcall level of ffa_init() to rootfs_initcall"),
> > which is being reverted in this patch set.
> >
> > Ordering within an initcall level needs to be fixed, but for now retrying at
> > late_initcall_sync works for some, hopefully most, cases.
>
> That's not a good answer. Ignoring the TPM issue for a moment, can
> you confirm that moving IMA's initialization to late-sync is safe? If
> not, why is this approach being considered?
I'm not seeing any measurements between late_initcall and late_initcall_sync.
Based on this very limited testing, I don't feel comfortable to actually move
the syscall, but can see adding support to allow IMA initialization to be
deferred.
Mimi
^ permalink raw reply
* Re: [RFC PATCH v2 1/4] security: ima: call ima_init() again at late_initcall_sync for defered TPM
From: Paul Moore @ 2026-04-24 22:10 UTC (permalink / raw)
To: Mimi Zohar
Cc: Yeoreum Yun, roberto.sassu, Jonathan McDowell,
linux-security-module, linux-kernel, linux-integrity,
linux-arm-kernel, kvmarm, jmorris, serge, dmitry.kasatkin,
eric.snowberg, jarkko, jgg, sudeep.holla, maz, oupton, joey.gouly,
suzuki.poulose, yuzenghui, catalin.marinas, will, noodles,
sebastianene
In-Reply-To: <1f78fc75b2e95239973612a4b6c4cc782960b7ac.camel@linux.ibm.com>
On Fri, Apr 24, 2026 at 6:01 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> On Fri, 2026-04-24 at 17:08 -0400, Paul Moore wrote:
> > On Fri, Apr 24, 2026 at 4:58 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> > > On Fri, 2026-04-24 at 16:15 -0400, Paul Moore wrote:
> > > > On Fri, Apr 24, 2026 at 1:57 AM Yeoreum Yun <yeoreum.yun@arm.com> wrote:
> > > > > > On Thu, Apr 23, 2026 at 2:13 PM Yeoreum Yun <yeoreum.yun@arm.com> wrote:
> > > > > > >
> > > > > > > Sounds good. Once the patch is posted, I’ll review it as well.
> > > > > > > Sorry again for the noise, and thanks for your patience ;)
> > > > > >
> > > > > > My apologies for not getting a chance to look at this patchset sooner.
> > > > > >
> > > > > > This seems like an obvious, perhaps even stupid, question, but I have
> > > > > > to ask: if IMA can be properly initialized via late_initcall_sync(),
> > > > > > why not simply do the initialization in late_initcall_sync() and drop
> > > > > > the late_initcall() initialization?
> > > > > >
> > > > > > Does any IMA functionality suffer if initialization waits until
> > > > > > late_initcall_sync()? If so, it seems non-critical if waiting until
> > > > > > _sync() is acceptable, as it appears in these patches/comments.
> > > > >
> > > > > This is the way first patch did, and here is some discussion for this
> > > > > (Might you have seen, but in case of you missed):
> > > > > - https://lore.kernel.org/all/a6a0e15286c983d720de227c6827adbe976c5b9b.camel@linux.ibm.com/
> > > >
> > > > Thanks for the pointer.
> > > >
> > > > Unfortunately, my concern remains the same: it's either "safe" to
> > > > initialize IMA at late_initcall_sync() or it isn't. Attempting to
> > > > initialize IMA twice seems both odd and wrong.
> > >
> > > Agreed. However, IMA should be initialized as soon as the TPM becomes
> > > available, not delayed.
> > >
> > > In patch 2/4 patch description, Jonathan describes the reasoning:
> > >
> > > Unfortunately some TPM drivers (such as Arm FF-A, or SPI attached TPM
> > > devices) are not reliably available during the initcall_late stage,
> > > resulting in a log error:
> > >
> > > ima: No TPM chip found, activating TPM-bypass!
> > >
> > > and no measurements into the TPM by IMA. We can avoid this by doing IMA
> > > init in the initcall_late_sync stage, after the drivers have completed
> > > their init + registration.
> > >
> > > Rather than do this everywhere, and needlessly delay the initialisation
> > > of IMA when there is no need to do so, we continue to try to initialise
> > > at the earlier stage, only deferring to the later point if the TPM is
> > > not available yet.
> >
> > Once again, that heavily implies that it is safe to initialize IMA in late-sync.
> >
> > Put another way, what breaks if IMA's initialization is delayed to
> > late-sync? If the answer is nothing, just move the initialization to
> > late-sync. However, if something *is* broken and we are just doing
> > this because of TPM delays at boot, this patchset just creates
> > additional problems and we need a different solution. I can't
> > envision a scenario where it makes sense to attempt initialization
> > twice.
> >
> > > > I understand the need to ensure that the TPM is available, but if it
> > > > isn't safe to wait to initialize IMA at late_initcall_sync() then it
> > > > would seem like this is a bad option and we need another mechanism to
> > > > synchronize IMA with TPM devices. If it is safe to initalize IMA in
> > > > late_initcall_sync(), just do that and be done with it.
> > >
> > > Within the same initcall level there is no way of ordering the initialization.
> > > Yeorum attempted to address the ordering issue in commit 0e0546eabcd6
> > > ("firmware: arm_ffa: Change initcall level of ffa_init() to rootfs_initcall"),
> > > which is being reverted in this patch set.
> > >
> > > Ordering within an initcall level needs to be fixed, but for now retrying at
> > > late_initcall_sync works for some, hopefully most, cases.
> >
> > That's not a good answer. Ignoring the TPM issue for a moment, can
> > you confirm that moving IMA's initialization to late-sync is safe? If
> > not, why is this approach being considered?
>
> I'm not seeing any measurements between late_initcall and late_initcall_sync.
> Based on this very limited testing, I don't feel comfortable to actually move
> the syscall, but can see adding support to allow IMA initialization to be
> deferred.
(I'm assuming you meant initcall and not syscall above, but if you're
talking about something else, please let me know.)
Saying that you aren't comfortable moving IMA initialization to
late-sync is inconsistent with allowing IMA initialization to be
deferred to late-sync. Either it is okay to initialize IMA in
late-sync or it isn't. You must pick one.
--
paul-moore.com
^ permalink raw reply
* [RFC PATCH net-next 0/3] net: macb: candidate fixes for silent TX stall on BCM2712/RP1
From: Lukasz Raczylo @ 2026-04-24 22:38 UTC (permalink / raw)
To: netdev
Cc: Nicolas Ferre, Claudiu Beznea, Andrew Lunn, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, linux-kernel,
linux-arm-kernel, linux-rpi-kernel
Hi netdev, Nicolas, Claudiu, linux-rpi,
This series proposes three candidate fixes for the silent TX stall
observed on Raspberry Pi 5 (BCM2712 SoC, Cadence GEM via RP1 PCIe
south bridge). The bug has been reported, with reproducers, at:
* https://github.com/cilium/cilium/issues/43198
* https://bugs.launchpad.net/ubuntu/+source/linux-raspi/+bug/2133877
Cilium #43198 reports reproduction on linux-raspi 6.17.0-1004, and
explicitly notes reproduction with both Cilium/eBPF and
Calico/nftables dataplanes (i.e. not CNI-specific). We observe the
same failure mode on kernel 6.18.24 built from raspberrypi/linux
rpi-6.18.y @ f2f68e79f16f, across a 24-node Raspberry Pi 5 fleet.
The 6.17/6.18 commonality and the two-CNI reproduction together
put the root cause below the packet-scheduling layer, in the macb
driver or the RP1 PCIe path.
Observed symptoms (our side, 6.18.24; consistent with the linked
reports):
* queue->tx_tail stops advancing at a single second;
* /sys/class/net/<iface>/statistics/tx_packets stops incrementing;
* qdisc backlog grows past zero; netif_stop_subqueue() is called;
* RX counters continue advancing; the MAC IRQ line continues to
fire (RX completions are handled);
* no kernel log line is produced for the duration of the stall;
* dev_watchdog does not fire: macb_netdev_ops has no
.ndo_tx_timeout, and our reading is that trans_start is kept
fresh by successful xmit prior to the ring filling;
* recovery on our side has required `ip link set <iface> down/up`
via an out-of-band watchdog DaemonSet.
Reading the current driver we identified three plausible races
between driver and hardware, each of which could independently
produce the observed behaviour. We did not determine which is the
actual root cause -- that likely requires either BCM2712/RP1
documentation we do not have, or dynamic tracing of the driver
during an in-situ stall. The series therefore attempts to close
all three, with each commit message stating which specific race
that patch is targeting.
Patch 1/3 -- flush PCIe posted write after TSTART doorbell.
Writes to NCR are posted PCIe writes and may not reach the MAC
before the driver returns. If the TSTART doorbell is lost, no
TX starts, no TCOMP arrives, and the ring goes quiescent. A
read-back of NCR after the write is a standard read-after-write
PCIe flush.
Patch 2/3 -- re-check ISR after IER re-enable in macb_tx_poll().
An existing comment in macb_tx_poll() notes that completions
raised while TCOMP is masked do not re-fire when IER is
re-enabled, and mitigates the window with macb_tx_complete_pending(),
which inspects driver-visible ring state only (after rmb()). On
PCIe-attached parts the descriptor DMA write that sets TX_USED
can remain in flight when that check runs; the rmb() orders CPU
writes but does not retire peripheral DMA. Reading ISR directly
after IER re-enable addresses this in two ways: (a) the MMIO read
is an architected PCIe read barrier for prior DMA writes, so a
subsequent macb_tx_complete_pending() sees up-to-date TX_USED
state; (b) it directly observes a pending TCOMP bit if the
hardware has one set. Either signal reschedules NAPI.
Patch 3/3 -- TX stall watchdog. Defence-in-depth. If patches
1 and 2 close the races we identified, this patch performs a
single spin_lock_irqsave/unlock and a branch per queue per
second with no other effect. If a further race remains that we
have not identified, it invokes the driver's own existing
macb_tx_restart(), which already verifies that TBQP is behind
tx_head before re-asserting TSTART. We include this patch
because we have empirically observed multi-minute stalls on this
hardware; we are willing to drop it if the preference is for
1 and 2 to stand alone.
Status and testing:
* Apply-tested against Linux net-next HEAD (this series is
generated from it) and against raspberrypi/linux rpi-6.18.y @
f2f68e79f16f (the fork our fleet runs): all three apply
cleanly on net-next; the rpi fork carries an additional local
`bool tx_pending` field on `struct macb_queue` that is not in
mainline, so we maintain a small rebased patch 3 hunk for it.
* Build-tested: the series compiles cleanly as part of our Talos
image build pipeline on arm64.
* Runtime-tested, early signal: ~4 h 20 min of post-patch uptime
on the canary node, ~3 h 15 min on the slowest (last master to
upgrade), ~95 node-hours cumulative across the 24-node fleet at
the time this cover letter was written. During that window the
fleet-wide counts are zero RECOVER events, zero `[tx-stall]`
partial markers (an out-of-band userspace detector that records
even transient one-second freezes that recover before the
3-second threshold), and zero ping-failure markers. Pre-patch
reference window (2026-04-24 14:00-18:10 UTC, when proper
monitoring was in place) observed multiple stalls per hour at
fleet level; at that rate we would expect on the order of 50
stalls in 95 node-hours, actual is zero. We will follow up
with a 24 h and a 1-week data point as the same observability
runs forward; the direction so far is consistent with patches
1 and 2 closing the underlying race(s) and patch 3 correctly
being a no-op on healthy hardware.
The series does not depend on any other in-flight work we are
aware of. Happy to split, rebase, or drop individual patches on
feedback. All three are independently revertable.
Lukasz Raczylo (3):
net: macb: flush PCIe posted write after TSTART doorbell
net: macb: re-check ISR after IER re-enable in macb_tx_poll
net: macb: add TX stall watchdog as defence-in-depth safety net
drivers/net/ethernet/cadence/macb.h | 5 ++
drivers/net/ethernet/cadence/macb_main.c | 99 +++++++++++++++++++++---
2 files changed, 94 insertions(+), 10 deletions(-)
--
2.53.0
^ permalink raw reply
* [RFC PATCH net-next 1/3] net: macb: flush PCIe posted write after TSTART doorbell
From: Lukasz Raczylo @ 2026-04-24 22:38 UTC (permalink / raw)
To: netdev
Cc: Nicolas Ferre, Claudiu Beznea, Andrew Lunn, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, linux-kernel,
linux-arm-kernel, linux-rpi-kernel
In-Reply-To: <cover.1777064117.git.lukasz@raczylo.com>
macb_start_xmit() and macb_tx_restart() kick transmission by
OR-ing MACB_BIT(TSTART) into NCR. On PCIe-attached macb instances
(BCM2712 + RP1 PCIe south bridge on Raspberry Pi 5 is the setup we
have in front of us), writes to NCR are posted PCIe writes: they
are not guaranteed to reach the device before the issuing CPU
returns. If the TSTART doorbell does not reach the MAC, no TX
begins, no TCOMP completion arrives, and the ring remains
quiescent without any kernel-visible indication.
Note that the raspberrypi/linux vendor fork carries a local patch
around the TSTART site (a queue->tx_pending breadcrumb that is
promoted to queue->txubr_pending by the next TCOMP interrupt,
triggering macb_tx_restart()). That workaround makes the loss
recoverable under traffic, but it cannot help if TCOMP itself is
not raised because no TX started -- which is exactly the case we
are targeting here. The handshake is not present in mainline.
Add a read-back of NCR after each TSTART write in macb_start_xmit()
and macb_tx_restart(). The read is an architected PCIe read
barrier for earlier posted writes on the same path; it ensures the
doorbell has reached the MAC before the functions return.
We do not yet have direct hardware evidence that TSTART is being
lost on the RP1 path (that would require a PCIe protocol analyser,
or at minimum a before/after counter on queue->tx_stall_last_tail
with and without this patch applied in isolation). This patch is
one of a three-patch series ("candidate fixes for silent TX stall
on BCM2712/RP1"); see the cover letter for context. We have
verified the series compiles and applies cleanly against mainline
HEAD and against raspberrypi/linux rpi-6.18.y @ f2f68e79f16f;
runtime verification is pending.
Link: https://github.com/cilium/cilium/issues/43198
Link: https://bugs.launchpad.net/ubuntu/+source/linux-raspi/+bug/2133877
Signed-off-by: Lukasz Raczylo <lukasz@raczylo.com>
---
drivers/net/ethernet/cadence/macb_main.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index a12aa2124..b6cca55ad 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -1922,6 +1922,13 @@ static void macb_tx_restart(struct macb_queue *queue)
spin_lock(&bp->lock);
macb_writel(bp, NCR, macb_readl(bp, NCR) | MACB_BIT(TSTART));
+ /*
+ * Flush the PCIe posted-write queue so the TSTART doorbell
+ * reliably reaches the MAC. Without this, the write can sit
+ * in the fabric and the MAC never advances, causing a silent
+ * TX stall.
+ */
+ (void)macb_readl(bp, NCR);
spin_unlock(&bp->lock);
out_tx_ptr_unlock:
@@ -2560,6 +2567,11 @@ static netdev_tx_t macb_start_xmit(struct sk_buff *skb, struct net_device *dev)
spin_lock(&bp->lock);
macb_tx_lpi_wake(bp);
macb_writel(bp, NCR, macb_readl(bp, NCR) | MACB_BIT(TSTART));
+ /*
+ * Flush the PCIe posted-write queue; see the comment in
+ * macb_tx_restart() for the reasoning.
+ */
+ (void)macb_readl(bp, NCR);
spin_unlock(&bp->lock);
if (CIRC_SPACE(queue->tx_head, queue->tx_tail, bp->tx_ring_size) < 1)
--
2.53.0
^ permalink raw reply related
* [RFC PATCH net-next 2/3] net: macb: re-check ISR after IER re-enable in macb_tx_poll
From: Lukasz Raczylo @ 2026-04-24 22:38 UTC (permalink / raw)
To: netdev
Cc: Nicolas Ferre, Claudiu Beznea, Andrew Lunn, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, linux-kernel,
linux-arm-kernel, linux-rpi-kernel
In-Reply-To: <cover.1777064117.git.lukasz@raczylo.com>
macb_tx_poll() runs with TCOMP masked, drains the TX ring, then
calls napi_complete_done() and re-enables TCOMP via IER. An
existing comment in the function notes:
/* Packet completions only seem to propagate to raise
* interrupts when interrupts are enabled at the time, so if
* packets were sent while interrupts were disabled,
* they will not cause another interrupt to be generated when
* interrupts are re-enabled.
*/
and mitigates this by calling macb_tx_complete_pending(), which
inspects driver-visible ring state (descriptor->ctrl, after rmb())
and reschedules NAPI if a completion is observable in memory.
On PCIe-attached parts (BCM2712 + RP1 on Raspberry Pi 5 is the
setup we have in front of us), the descriptor DMA write that sets
TX_USED may not have retired to system memory at the point
macb_tx_complete_pending() runs. The rmb() synchronises the CPU
view of earlier CPU writes; it is not sufficient to retire an
in-flight peripheral DMA write. Under that ordering the in-memory
descriptor can still read TX_USED=0 when the hardware has in fact
completed the frame; the check returns false; NAPI exits; the
quirk above prevents the re-enabled IER from re-firing; the ring
goes quiescent.
Add an explicit ISR read after the IER write. The MMIO read
serves two independent purposes:
(1) It is an architected PCIe read barrier for earlier
peripheral-originated DMA writes on the same path, so a
subsequent macb_tx_complete_pending() observes any TX_USED
write that was in flight at the time of the barrier.
(2) It samples the hardware ISR directly, so a TCOMP bit that
the hardware set while TCOMP was masked is visible here,
independently of whether the descriptor DMA has retired.
If either signal indicates pending work, reschedule NAPI via the
same path as the existing check.
This patch addresses one of three candidate races for the silent
TX stall described in the cover letter. Whether it is sufficient
by itself, or whether it requires the PCIe posted-write flush in
patch 1/3 to cover the observed behaviour, we have not yet
verified at runtime.
Link: https://github.com/cilium/cilium/issues/43198
Link: https://bugs.launchpad.net/ubuntu/+source/linux-raspi/+bug/2133877
Signed-off-by: Lukasz Raczylo <lukasz@raczylo.com>
---
drivers/net/ethernet/cadence/macb_main.c | 28 +++++++++++++++---------
1 file changed, 18 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index b6cca55ad..ea231b1c5 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -1973,17 +1973,25 @@ static int macb_tx_poll(struct napi_struct *napi, int budget)
if (work_done < budget && napi_complete_done(napi, work_done)) {
queue_writel(queue, IER, MACB_BIT(TCOMP));
- /* Packet completions only seem to propagate to raise
- * interrupts when interrupts are enabled at the time, so if
- * packets were sent while interrupts were disabled,
- * they will not cause another interrupt to be generated when
- * interrupts are re-enabled.
- * Check for this case here to avoid losing a wakeup. This can
- * potentially race with the interrupt handler doing the same
- * actions if an interrupt is raised just after enabling them,
- * but this should be harmless.
+ /*
+ * TCOMP events that fire while the interrupt is masked do
+ * not re-fire when IER is re-enabled. Catch this two ways
+ * to avoid losing a wakeup:
+ *
+ * (1) Read ISR -- catches completions the hardware flagged
+ * but that we did not see as an interrupt. The MMIO
+ * read doubles as a PCIe read barrier, flushing any
+ * in-flight descriptor TX_USED DMA writes into memory.
+ * (2) macb_tx_complete_pending() inspects the ring after
+ * that flush, catching a descriptor whose TX_USED is
+ * now visible as a result of the barrier.
+ *
+ * This can race with the interrupt handler taking the same
+ * path if an interrupt fires just after the IER write;
+ * rescheduling NAPI in that case is harmless.
*/
- if (macb_tx_complete_pending(queue)) {
+ if ((queue_readl(queue, ISR) & MACB_BIT(TCOMP)) ||
+ macb_tx_complete_pending(queue)) {
queue_writel(queue, IDR, MACB_BIT(TCOMP));
macb_queue_isr_clear(bp, queue, MACB_BIT(TCOMP));
netdev_vdbg(bp->dev, "TX poll: packets pending, reschedule\n");
--
2.53.0
^ permalink raw reply related
* [RFC PATCH net-next 3/3] net: macb: add TX stall watchdog as defence-in-depth safety net
From: Lukasz Raczylo @ 2026-04-24 22:38 UTC (permalink / raw)
To: netdev
Cc: Nicolas Ferre, Claudiu Beznea, Andrew Lunn, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, linux-kernel,
linux-arm-kernel, linux-rpi-kernel
In-Reply-To: <cover.1777064117.git.lukasz@raczylo.com>
Patches 1/3 and 2/3 address two candidate races that could lead
to a TCOMP completion being missed on PCIe-attached macb
instances. This patch adds a defence-in-depth safety net, in
case a further race remains that we have not identified.
The watchdog is a per-queue delayed_work that runs once per
second. It snapshots queue->tx_tail; if the ring is non-empty
(queue->tx_head != queue->tx_tail) and tx_tail has not advanced
since the previous tick, it calls macb_tx_restart().
No new recovery logic is introduced. macb_tx_restart() already
exists in this file, is correctly locked (tx_ptr_lock, bp->lock),
and verifies that the hardware's TBQP is behind the driver's
head index before re-asserting TSTART. On a healthy ring it is
a no-op at the hardware level; the watchdog only supplies the
missing trigger.
On a healthy queue the per-tick cost is one spin_lock_irqsave()
/ spin_unlock_irqrestore() and one branch. The delayed_work is
only scheduled between macb_open() and macb_close(), and is
cancelled synchronously on close.
Context for submission: on our 24-node Raspberry Pi 5 fleet,
before this series, an out-of-band user-space watchdog
(monitoring tx_packets from /sys/class/net/.../statistics and
toggling the link down/up when it froze) was required to keep
nodes usable. We include this kernel-side watchdog as a cleaner
in-kernel equivalent for any residual stall that patches 1 and
2 do not cover. We are willing to drop this patch if the view
is that 1 and 2 should stand alone.
Link: https://github.com/cilium/cilium/issues/43198
Link: https://bugs.launchpad.net/ubuntu/+source/linux-raspi/+bug/2133877
Signed-off-by: Lukasz Raczylo <lukasz@raczylo.com>
---
drivers/net/ethernet/cadence/macb.h | 5 ++
drivers/net/ethernet/cadence/macb_main.c | 59 ++++++++++++++++++++++++
2 files changed, 64 insertions(+)
diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index 2de56017e..9115f2b47 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -1278,6 +1278,11 @@ struct macb_queue {
dma_addr_t tx_ring_dma;
struct work_struct tx_error_task;
bool txubr_pending;
+
+ /* TX stall watchdog -- see macb_tx_stall_watchdog() in macb_main.c */
+ struct delayed_work tx_stall_watchdog_work;
+ unsigned int tx_stall_last_tail;
+
struct napi_struct napi_tx;
dma_addr_t rx_ring_dma;
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index ea231b1c5..ea2306ef7 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -2002,6 +2002,59 @@ static int macb_tx_poll(struct napi_struct *napi, int budget)
return work_done;
}
+#define MACB_TX_STALL_INTERVAL_MS 1000
+
+/*
+ * TX stall watchdog.
+ *
+ * Defence-in-depth against lost TCOMP interrupts. macb already has a
+ * recovery chain (tx_pending -> txubr_pending -> macb_tx_restart())
+ * that fires on TCOMP; if TCOMP itself is lost the TX ring stalls
+ * silently until something else kicks TSTART. This watchdog runs
+ * once per second per queue, snapshots tx_tail, and calls
+ * macb_tx_restart() if the ring is non-empty and tx_tail has not
+ * advanced since the previous tick.
+ *
+ * macb_tx_restart() already checks the hardware's TBQP against the
+ * driver's head index before re-asserting TSTART, so on a healthy
+ * ring this is a no-op at the hardware level. The watchdog only
+ * adds the missing trigger.
+ */
+static void macb_tx_stall_watchdog(struct work_struct *work)
+{
+ struct macb_queue *queue = container_of(to_delayed_work(work),
+ struct macb_queue,
+ tx_stall_watchdog_work);
+ struct macb *bp = queue->bp;
+ unsigned int cur_tail, cur_head;
+ bool stalled = false;
+ unsigned long flags;
+
+ if (!netif_running(bp->dev))
+ return;
+
+ spin_lock_irqsave(&queue->tx_ptr_lock, flags);
+ cur_tail = queue->tx_tail;
+ cur_head = queue->tx_head;
+ if (cur_head != cur_tail &&
+ cur_tail == queue->tx_stall_last_tail)
+ stalled = true;
+ else
+ queue->tx_stall_last_tail = cur_tail;
+ spin_unlock_irqrestore(&queue->tx_ptr_lock, flags);
+
+ if (stalled) {
+ netdev_warn_once(bp->dev,
+ "TX stall detected on queue %u (tail=%u head=%u); re-kicking TSTART\n",
+ (unsigned int)(queue - bp->queues),
+ cur_tail, cur_head);
+ macb_tx_restart(queue);
+ }
+
+ schedule_delayed_work(&queue->tx_stall_watchdog_work,
+ msecs_to_jiffies(MACB_TX_STALL_INTERVAL_MS));
+}
+
static void macb_hresp_error_task(struct work_struct *work)
{
struct macb *bp = from_work(bp, work, hresp_err_bh_work);
@@ -3190,6 +3243,9 @@ static int macb_open(struct net_device *dev)
for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
napi_enable(&queue->napi_rx);
napi_enable(&queue->napi_tx);
+ queue->tx_stall_last_tail = queue->tx_tail;
+ schedule_delayed_work(&queue->tx_stall_watchdog_work,
+ msecs_to_jiffies(MACB_TX_STALL_INTERVAL_MS));
}
macb_init_hw(bp);
@@ -3240,6 +3296,7 @@ static int macb_close(struct net_device *dev)
for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
napi_disable(&queue->napi_rx);
napi_disable(&queue->napi_tx);
+ cancel_delayed_work_sync(&queue->tx_stall_watchdog_work);
netdev_tx_reset_queue(netdev_get_tx_queue(dev, q));
}
@@ -4802,6 +4859,8 @@ static int macb_init_dflt(struct platform_device *pdev)
}
INIT_WORK(&queue->tx_error_task, macb_tx_error_task);
+ INIT_DELAYED_WORK(&queue->tx_stall_watchdog_work,
+ macb_tx_stall_watchdog);
q++;
}
--
2.53.0
^ permalink raw reply related
* Re: [RFC PATCH v2 1/4] security: ima: call ima_init() again at late_initcall_sync for defered TPM
From: Mimi Zohar @ 2026-04-24 22:49 UTC (permalink / raw)
To: Paul Moore
Cc: Yeoreum Yun, roberto.sassu, Jonathan McDowell,
linux-security-module, linux-kernel, linux-integrity,
linux-arm-kernel, kvmarm, jmorris, serge, dmitry.kasatkin,
eric.snowberg, jarkko, jgg, sudeep.holla, maz, oupton, joey.gouly,
suzuki.poulose, yuzenghui, catalin.marinas, will, noodles,
sebastianene
In-Reply-To: <CAHC9VhS4JmPmCJrYTdbjxb3TO-uK6cB3Zij-ef=wswGce2BGzg@mail.gmail.com>
On Fri, 2026-04-24 at 18:10 -0400, Paul Moore wrote:
> (I'm assuming you meant initcall and not syscall above, but if you're
> talking about something else, please let me know.)
>
> Saying that you aren't comfortable moving IMA initialization to
> late-sync is inconsistent with allowing IMA initialization to be
> deferred to late-sync. Either it is okay to initialize IMA in
> late-sync or it isn't. You must pick one.
Yes, we're discussing late_initcall and late_initcall_sync.
I prefer to look at it as being pragmatic. I'd rather err on the side of caution
and not move the syscall to late_initcall_sync, than move it. However, others
have moved the syscall to address the TPM-bypass issue for their environment.
Mimi
^ permalink raw reply
* Re: [PATCH 1/1] Revert "scsi: ufs: Use pre-calculated offsets in ufshcd_init_lrb()"
From: Bart Van Assche @ 2026-04-24 22:55 UTC (permalink / raw)
To: ed.tsai, Alim Akhtar, James E.J. Bottomley, Martin K. Petersen,
Matthias Brugger, AngeloGioacchino Del Regno, Avri Altman
Cc: linux-kernel, linux-arm-kernel, linux-mediatek, wsd_upstream,
peter.wang, alice.chao, naomi.chu, chun-hung.wu, linux-scsi
In-Reply-To: <20260424063603.382328-2-ed.tsai@mediatek.com>
On 4/23/26 11:35 PM, ed.tsai@mediatek.com wrote:
> Note that these DMA addresses are only used in ufshcd_print_tr() for
> error logging, so the impact is limited to misleading error logs.
Instead of fixing these offsets, please remove the ucd_rsp_dma_addr and
ucd_prdt_dma_addr members from struct ufshcd_lrb.
Thanks,
Bart.
^ permalink raw reply
* Re: [PATCH v3 05/11] iommu: Change group->devices to RCU-protected list
From: Jason Gunthorpe @ 2026-04-24 22:58 UTC (permalink / raw)
To: Nicolin Chen
Cc: Baolu Lu, Will Deacon, Robin Murphy, Joerg Roedel, Bjorn Helgaas,
Rafael J . Wysocki, Len Brown, Pranjal Shrivastava, Mostafa Saleh,
Kevin Tian, linux-arm-kernel, iommu, linux-kernel, linux-acpi,
linux-pci, vsethi, Shuai Xue
In-Reply-To: <aevAqKFq7ATYk3+i@Asurada-Nvidia>
On Fri, Apr 24, 2026 at 12:12:40PM -0700, Nicolin Chen wrote:
> On Fri, Apr 24, 2026 at 10:11:48AM -0300, Jason Gunthorpe wrote:
> > On Thu, Apr 23, 2026 at 08:08:59PM -0700, Nicolin Chen wrote:
> > > On Fri, Apr 24, 2026 at 10:53:49AM +0800, Baolu Lu wrote:
> > > > On 4/17/26 07:28, Nicolin Chen wrote:
> > > > mutex_unlock(&group->mutex);
> > > > /*
> > > > * FIXME: Mis-locked because the ops->probe_finalize() call-back
> > > > * of some IOMMU drivers calls arm_iommu_attach_device() which
> > > > * in-turn might call back into IOMMU core code, where it tries
> > > > * to take group->mutex, resulting in a deadlock.
> > > > */
> > > > for_each_group_device(group, gdev)
> > > > iommu_group_do_probe_finalize(gdev->dev);
> > > > }
> > > >
> > > > return 0;
> > > > }
> > > >
> > > > Will the change above trigger a lockdep splat due to this "mis-locked"
> > > > case?"
> > >
> > > Oh, I missed this one. That's a good finding!
> > >
> > > Perhaps we can just change it to list_for_each_entry_rcu holding
> > > rcu_read_lock() and drop the FIXME.
> >
> > You can't hold rcu across that function IIRC
>
> Oh right. I didn't think too carefully...
>
> I tend to keep it as-is. So, maybe just list_for_each_entry?
Does your series make this existing race materially worse?
Jason
^ permalink raw reply
* Re: [RFC PATCH 2/5] arm64: dts: rockchip: add ISP nodes to rk3588
From: Laurent Pinchart @ 2026-04-24 23:00 UTC (permalink / raw)
To: Paul Elder
Cc: Xu Hongfei, michael.riesch, stefan.klug, linux-media,
linux-arm-kernel, linux-rockchip, linux-kernel, Heiko Stuebner
In-Reply-To: <20260424175853.638202-3-paul.elder@ideasonboard.com>
Hi Paul,
Thank you for the patch.
On Sat, Apr 25, 2026 at 02:58:47AM +0900, Paul Elder wrote:
> From: Xu Hongfei <xuhf@rock-chips.com>
>
> Add device tree nodes for the ISP and their iommus on the RK3588.
>
> Signed-off-by: Xu Hongfei <xuhf@rock-chips.com>
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> ---
> arch/arm64/boot/dts/rockchip/rk3588-base.dtsi | 60 +++++++++++++++++++
> 1 file changed, 60 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
> index 8b98e5c3cc8b..607b03d55dfd 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
> @@ -3535,6 +3535,66 @@ gpio4: gpio@fec50000 {
> #interrupt-cells = <2>;
> };
> };
> +
> + isp0: isp@fdcb0000 {
> + compatible = "rockchip,rk3588-isp";
> + reg = <0x0 0xfdcb0000 0x0 0x7f00>;
> + interrupts = <GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH 0>,
> + <GIC_SPI 133 IRQ_TYPE_LEVEL_HIGH 0>,
> + <GIC_SPI 134 IRQ_TYPE_LEVEL_HIGH 0>;
> + interrupt-names = "isp_irq", "mi_irq";
> + clocks = <&cru ACLK_ISP0>, <&cru HCLK_ISP0>,
> + <&cru CLK_ISP0_CORE>, <&cru CLK_ISP0_CORE_MARVIN>,
> + <&cru CLK_ISP0_CORE_VICAP>;
> + clock-names = "aclk", "hclk", "clk_core",
> + "clk_core_marvin", "clk_core_vicap";
> + power-domains = <&power RK3588_PD_VI>;
> + iommus = <&isp0_mmu>;
> + status = "disabled";
> + };
> +
> + isp0_mmu: iommu@fdcb7f00 {
> + compatible = "rockchip,rk3588-iommu", "rockchip,rk3568-iommu";
> + reg = <0x0 0xfdcb7f00 0x0 0x100>;
> + interrupts = <GIC_SPI 132 IRQ_TYPE_LEVEL_HIGH 0>;
> + interrupt-names = "isp0_mmu";
I don't think interrupt-names is needed. Same for the second IOMMU.
> + clocks = <&cru ACLK_ISP0>, <&cru HCLK_ISP0>;
> + clock-names = "aclk", "iface";
> + power-domains = <&power RK3588_PD_VI>;
> + #iommu-cells = <0>;
> + rockchip,disable-mmu-reset;
> + status = "disabled";
> + };
> +
> + isp1: isp@fdcc0000 {
> + compatible = "rockchip,rk3588-isp";
> + reg = <0x0 0xfdcc0000 0x0 0x7f00>;
> + interrupts = <GIC_SPI 135 IRQ_TYPE_LEVEL_HIGH 0>,
> + <GIC_SPI 137 IRQ_TYPE_LEVEL_HIGH 0>,
> + <GIC_SPI 138 IRQ_TYPE_LEVEL_HIGH 0>;
> + interrupt-names = "isp_irq", "mi_irq";
> + clocks = <&cru ACLK_ISP1>, <&cru HCLK_ISP1>,
> + <&cru CLK_ISP1_CORE>, <&cru CLK_ISP1_CORE_MARVIN>,
> + <&cru CLK_ISP1_CORE_VICAP>;
> + clock-names = "aclk", "hclk", "clk_core",
> + "clk_core_marvin", "clk_core_vicap";
> + power-domains = <&power RK3588_PD_ISP1>;
> + iommus = <&isp1_mmu>;
> + status = "disabled";
> + };
> +
> + isp1_mmu: iommu@fdcc7f00 {
> + compatible = "rockchip,rk3588-iommu", "rockchip,rk3568-iommu";
> + reg = <0x0 0xfdcc7f00 0x0 0x100>;
> + interrupts = <GIC_SPI 136 IRQ_TYPE_LEVEL_HIGH 0>;
> + interrupt-names = "isp1_mmu";
> + clocks = <&cru ACLK_ISP1>, <&cru HCLK_ISP1>;
> + clock-names = "aclk", "iface";
> + power-domains = <&power RK3588_PD_ISP1>;
> + #iommu-cells = <0>;
> + rockchip,disable-mmu-reset;
> + status = "disabled";
> + };
> };
>
> #include "rk3588-base-pinctrl.dtsi"
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH RFC bpf-next 0/8] bpf: add support for KASAN checks in JITed programs
From: Ihor Solodrai @ 2026-04-24 23:10 UTC (permalink / raw)
To: Alexis Lothoré (eBPF Foundation), Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau,
Eduard Zingerman, Kumar Kartikeya Dwivedi, Song Liu,
Yonghong Song, Jiri Olsa, John Fastabend, David S. Miller,
David Ahern, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H. Peter Anvin, Shuah Khan, Maxime Coquelin,
Alexandre Torgue, Andrey Ryabinin, Alexander Potapenko,
Andrey Konovalov, Dmitry Vyukov, Vincenzo Frascino, Andrew Morton
Cc: ebpf, Bastien Curutchet, Thomas Petazzoni, Xu Kuohai, bpf,
linux-kernel, netdev, linux-kselftest, linux-stm32,
linux-arm-kernel, kasan-dev, linux-mm
In-Reply-To: <20260413-kasan-v1-0-1a5831230821@bootlin.com>
On 4/13/26 11:28 AM, Alexis Lothoré (eBPF Foundation) wrote:
> Hello,
> this series aims to bring basic support for KASAN checks to BPF JITed
> programs. This follows the first RFC posted in [1].
Hi Alexis,
Thank you for working on this, it's a real testing gap.
I have a few comments, see below.
The series doesn't apply cleanly on bpf-next right now, but I was able
to apply to a little older revision (eb5249b12507).
>
> KASAN allows to spot memory management mistakes by reserving a fraction
> of memory as "shadow memory" that will map to the rest of the memory and
> allow its monitoring. Each memory-accessing instruction is then
> instrumented at build time to call some ASAN check function, that will
> analyze the corresponding bits in shadow memory, and if it detects the
> access as invalid, trigger a detailed report. The goal of this series is
> to replicate this mechanism for BPF programs when they are being JITed
> into native instructions: that's then the (runtime) JIT compiler who is
> in charge of inserting calls to the corresponding kasan checks, when a
> program is being loaded into the kernel. This task involves:
> - identifying at program load time the instructions performing memory
> accesses
> - identifying those accesses properties (size ? read or write ?) to
> define the relevant kasan check function to call
> - just before the identified instructions:
> - perform the basic context saving (ie: saving registers)
> - inserting a call to the relevant kasan check function
> - restore context
> - whenever the instrumented program executes, if it performs an invalid
> access, it triggers a kasan report identical to those instrumented on
> kernel side at build time.
>
> As discussed in [1], this series is based on some choices and
> assumptions:
> - it focuses on x86_64 for now, and so only on KASAN_GENERIC
I wonder if it's feasible to implement KASAN support on the verifier
side in post-verification fixups. AI slop for illustration:
;; Original (1 BPF insn):
dst = *(u64 *)(src + off) ; BPF_LDX | BPF_MEM | BPF_DW
;; Rewrite (~7 BPF insns):
r_tmp1 = src ; BPF_MOV64_REG
r_tmp1 += off ; BPF_ALU64 | BPF_ADD | K (full address)
r_tmp2 = r_tmp1 ; copy
r_tmp2 >>= 3 ; KASAN_SHADOW_SCALE_SHIFT
r_tmp2 += KASAN_SHADOW_OFFSET ; shadow address
r_tmp3 = *(u8 *)(r_tmp2 + 0) ; BPF_LDX | BPF_B (load shadow byte)
if r_tmp3 != 0 goto +2 ; BPF_JNE | PC+2
dst = *(u64 *)(src + off) ; original access (fast path)
goto +1 ; skip slowpath
call __asan_report_load8 ; BPF kfunc
dst = *(u64 *)(src + off) ; retry the access after report (non-fatal)
A sort of inline kasan directly in BPF.
There are plenty of issues with it: instruction limit, exposing asan
API as kfuncs, etc. On the flip side we get cross-arch support out of
the box with no or mininal JIT changes.
Honestly I'm not excited about this approach, but curious if anyone
thought about this, or maybe it was already discussed?
> - not all memory accessing BPF instructions are being instrumented:
> - it focuses on STX/LDX instructions
> - it discards instructions accessing BPF program stack (already
> monitored by page guards)
> - it discards possibly faulting instructions, like BPF_PROBE_MEM or
> BPF_PROBE_ATOMIC insns
>
> The series is marked and sent as RFC:
> - to allow collecting feedback early and make sure that it goes into the
> right direction
> - because it depends on Xu's work to pass data between the verifier and
> JIT compilers. This work is not merged yet, see [2]. I have been
> tracking the various revisions he sent on the ML and based my local
> branch on his work
> - because tests brought by this series currently can't run on BPF CI:
> they expect kasan multishot to be enabled, otherwise the first test
> will make all other kasan-related tests fail.
AFAICT this can be trivially fixed on BPF CI side, we just need to set
kasan_multi_shot for the VMs running the tests. I will do that, your
next revision doesn't have to be and RFC.
> - because some cases like atomic loads/stores are not instrumented yet
> (and are still making me scratch my head)
> - because it will hopefully provide a good basis to discuss the topic at
> LSFMMBPF (see [3])
Apparently, KASAN reporting routine takes a lock [1]:
__asan_load()
-> check_region_inline()
-> kasan_report()
-> start_report()
-> raw_spin_lock_irqsave(&report_lock, *flags);
BPF programs can run in NMI context, and so it appears to be possible
to get an unflagged (because of lockdep_off() in start_report)
deadlock, if an NMI fires on a CPU already holding report_lock.
Although I guess you'd need two KASAN bugs to happen
simultaneously for that to occur?... A rare event, I would hope.
It could be addressed with either in_nmi() check at runtime, or
forbidding kasan for NMI-runnable BPF program types.
That said, this may be a case of being overly defensive to appease the
ai overlords.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/mm/kasan/report.c?h=v7.0#n204
>
> Despite this series not being ready for integration yet, anyone
> interested in running it locally can perform the following steps to run
> the JITed KASAN instrumentation selftests:
> - rebasing locally this series on [2]
> - building and running the corresponding kernel with kasan_multi_shot
> enabled
> - running `test_progs -a kasan`
>
> And should get a variety of KASAN tests executed for BPF programs:
>
> #162/1 kasan/bpf_kasan_uaf_read_1:OK
> #162/2 kasan/bpf_kasan_uaf_read_2:OK
> #162/3 kasan/bpf_kasan_uaf_read_4:OK
> #162/4 kasan/bpf_kasan_uaf_read_8:OK
> #162/5 kasan/bpf_kasan_uaf_write_1:OK
> #162/6 kasan/bpf_kasan_uaf_write_2:OK
> #162/7 kasan/bpf_kasan_uaf_write_4:OK
> #162/8 kasan/bpf_kasan_uaf_write_8:OK
> #162/9 kasan/bpf_kasan_oob_read_1:OK
> #162/10 kasan/bpf_kasan_oob_read_2:OK
> #162/11 kasan/bpf_kasan_oob_read_4:OK
> #162/12 kasan/bpf_kasan_oob_read_8:OK
> #162/13 kasan/bpf_kasan_oob_write_1:OK
> #162/14 kasan/bpf_kasan_oob_write_2:OK
> #162/15 kasan/bpf_kasan_oob_write_4:OK
> #162/16 kasan/bpf_kasan_oob_write_8:OK
> #162 kasan:OK
> Summary: 1/16 PASSED, 0 SKIPPED, 0 FAILED
>
> [1] https://lore.kernel.org/bpf/DG7UG112AVBC.JKYISDTAM30T@bootlin.com/
> [2] https://lore.kernel.org/bpf/cover.1776062885.git.xukuohai@hotmail.com/
> [3] https://lore.kernel.org/bpf/DGGNCXX79H8O.2P6K8L1QW1M8K@bootlin.com/
>
> Signed-off-by: Alexis Lothoré (eBPF Foundation) <alexis.lothore@bootlin.com>
> ---
> Alexis Lothoré (eBPF Foundation) (8):
> kasan: expose generic kasan helpers
> bpf: mark instructions accessing program stack
> bpf: add BPF_JIT_KASAN for KASAN instrumentation of JITed programs
> bpf, x86: add helper to emit kasan checks in x86 JITed programs
> bpf, x86: emit KASAN checks into x86 JITed programs
> selftests/bpf: do not run verifier JIT tests when BPF_JIT_KASAN is enabled
> bpf, x86: enable KASAN for JITed programs on x86
> selftests/bpf: add tests to validate KASAN on JIT programs
>
> arch/x86/Kconfig | 1 +
> arch/x86/net/bpf_jit_comp.c | 106 +++++++++++++
> include/linux/bpf.h | 2 +
> include/linux/bpf_verifier.h | 2 +
> include/linux/kasan.h | 13 ++
> kernel/bpf/Kconfig | 9 ++
> kernel/bpf/core.c | 10 ++
> kernel/bpf/verifier.c | 7 +
> mm/kasan/kasan.h | 10 --
> tools/testing/selftests/bpf/prog_tests/kasan.c | 165 +++++++++++++++++++++
> tools/testing/selftests/bpf/progs/kasan.c | 146 ++++++++++++++++++
> .../testing/selftests/bpf/test_kmods/bpf_testmod.c | 79 ++++++++++
> tools/testing/selftests/bpf/test_loader.c | 5 +
> tools/testing/selftests/bpf/unpriv_helpers.c | 5 +
> tools/testing/selftests/bpf/unpriv_helpers.h | 1 +
> 15 files changed, 551 insertions(+), 10 deletions(-)
> ---
> base-commit: 7990a071b32887a1a883952e8cf60134b6d6fea0
> change-id: 20260126-kasan-fcd68f64cd7b
>
> Best regards,
> --
> Alexis Lothoré (eBPF Foundation) <alexis.lothore@bootlin.com>
>
^ permalink raw reply
* Re: [PATCH RFC bpf-next 2/8] bpf: mark instructions accessing program stack
From: Ihor Solodrai @ 2026-04-24 23:18 UTC (permalink / raw)
To: Alexis Lothoré (eBPF Foundation), Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau,
Eduard Zingerman, Kumar Kartikeya Dwivedi, Song Liu,
Yonghong Song, Jiri Olsa, John Fastabend, David S. Miller,
David Ahern, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H. Peter Anvin, Shuah Khan, Maxime Coquelin,
Alexandre Torgue, Andrey Ryabinin, Alexander Potapenko,
Andrey Konovalov, Dmitry Vyukov, Vincenzo Frascino, Andrew Morton
Cc: ebpf, Bastien Curutchet, Thomas Petazzoni, Xu Kuohai, bpf,
linux-kernel, netdev, linux-kselftest, linux-stm32,
linux-arm-kernel, kasan-dev, linux-mm
In-Reply-To: <20260413-kasan-v1-2-1a5831230821@bootlin.com>
On 4/13/26 11:28 AM, Alexis Lothoré (eBPF Foundation) wrote:
> In order to prepare to emit KASAN checks in JITed programs, JIT
> compilers need to be aware about whether some load/store instructions
> are targeting the bpf program stack, as those should not be monitored
> (we already have guard pages for that, and it is difficult anyway to
> correctly monitor any kind of data passed on stack).
>
> To support this need, make the BPF verifier mark the instructions that
> access program stack:
> - add a setter that allows the verifier to mark instructions accessing
> the program stack
> - add a getter that allows JIT compilers to check whether instructions
> being JITed are accessing the stack
>
> Signed-off-by: Alexis Lothoré (eBPF Foundation) <alexis.lothore@bootlin.com>
> ---
> include/linux/bpf.h | 2 ++
> include/linux/bpf_verifier.h | 2 ++
> kernel/bpf/core.c | 10 ++++++++++
> kernel/bpf/verifier.c | 7 +++++++
> 4 files changed, 21 insertions(+)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index b4b703c90ca9..774a0395c498 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -1543,6 +1543,8 @@ void bpf_jit_uncharge_modmem(u32 size);
> bool bpf_prog_has_trampoline(const struct bpf_prog *prog);
> bool bpf_insn_is_indirect_target(const struct bpf_verifier_env *env, const struct bpf_prog *prog,
> int insn_idx);
> +bool bpf_insn_accesses_stack(const struct bpf_verifier_env *env,
> + const struct bpf_prog *prog, int insn_idx);
> #else
> static inline int bpf_trampoline_link_prog(struct bpf_tramp_link *link,
> struct bpf_trampoline *tr,
> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
> index b148f816f25b..ab99ed4c4227 100644
> --- a/include/linux/bpf_verifier.h
> +++ b/include/linux/bpf_verifier.h
> @@ -660,6 +660,8 @@ struct bpf_insn_aux_data {
> u16 const_reg_map_mask;
> u16 const_reg_subprog_mask;
> u32 const_reg_vals[10];
> + /* instruction accesses stack */
> + bool accesses_stack;
> };
>
> #define MAX_USED_MAPS 64 /* max number of maps accessed by one eBPF program */
> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> index 8b018ff48875..340abfdadbed 100644
> --- a/kernel/bpf/core.c
> +++ b/kernel/bpf/core.c
> @@ -1582,6 +1582,16 @@ bool bpf_insn_is_indirect_target(const struct bpf_verifier_env *env, const struc
> insn_idx += prog->aux->subprog_start;
> return env->insn_aux_data[insn_idx].indirect_target;
> }
> +
> +bool bpf_insn_accesses_stack(const struct bpf_verifier_env *env,
> + const struct bpf_prog *prog, int insn_idx)
> +{
> + if (!env)
> + return false;
> + insn_idx += prog->aux->subprog_start;
> + return env->insn_aux_data[insn_idx].accesses_stack;
> +}
> +
> #endif /* CONFIG_BPF_JIT */
>
> /* Base function for offset calculation. Needs to go into .text section,
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 1e36b9e91277..7bce4fb4e540 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -3502,6 +3502,11 @@ static void mark_indirect_target(struct bpf_verifier_env *env, int idx)
> env->insn_aux_data[idx].indirect_target = true;
> }
>
> +static void mark_insn_accesses_stack(struct bpf_verifier_env *env, int idx)
> +{
> + env->insn_aux_data[idx].accesses_stack = true;
> +}
> +
> #define LR_FRAMENO_BITS 3
> #define LR_SPI_BITS 6
> #define LR_ENTRY_BITS (LR_SPI_BITS + LR_FRAMENO_BITS + 1)
> @@ -6490,6 +6495,8 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
> else
> err = check_stack_write(env, regno, off, size,
> value_regno, insn_idx);
> +
> + mark_insn_accesses_stack(env, insn_idx);
I am not sure this can be done unconditionally here.
It may be possible in different states to have different pointer
types for the affected reg (PTR_TO_STACK in one execution path and say
PTR_TO_MAP_VALUE in another). And if set uncoditionally,
instrumentation may be skipped for legitimate targets.
Maybe reset by default in check_mem_access()?
> } else if (reg_is_pkt_pointer(reg)) {
> if (t == BPF_WRITE && !may_access_direct_pkt_data(env, NULL, t)) {
> verbose(env, "cannot write into packet\n");
>
^ permalink raw reply
* [PATCH 0/3] media: imx8-isi: fix resource lifecycle bugs
From: Xiaolei Wang @ 2026-04-24 23:19 UTC (permalink / raw)
To: laurent.pinchart, mchehab, Frank.Li, s.hauer, kernel, festevam,
aisheng.dong, jacopo, guoniu.zhou, s.riedmueller, Xiaolei.Wang
Cc: linux-media, imx, linux-arm-kernel, linux-kernel, stable
This series fixes three resource lifecycle issues in the imx8-isi driver,
all introduced by commit cf21f328fcaf ("media: nxp: Add i.MX8 ISI driver").
Patch 1 fixes a use-after-free on rmmod: mxc_isi_remove() called
crossbar cleanup before v4l2 cleanup, freeing the crossbar pads while
the media framework still needed them to remove links. Fix by swapping
the cleanup order.
Patch 2 fixes a memory leak on rmmod: both crossbar and pipe cleanup
paths were missing v4l2_subdev_cleanup() calls to free the subdev
active state allocated by v4l2_subdev_init_finalize().
Patch 3 fixes resource leaks in probe error paths: the pipes array
allocated with kzalloc_objs() was never freed on failure or remove,
and already-initialized pipes were not cleaned up when a later pipe
init or v4l2 init failed. Fix by switching to devm_kcalloc() and
adding pipe cleanup in the error path.
Xiaolei Wang (3):
media: imx8-isi: fix use-after-free on remove
media: imx8-isi: add missing v4l2_subdev_cleanup() in crossbar and
pipe
media: imx8-isi: fix resource leaks in probe error paths and remove
drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c | 7 +++++--
drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c | 1 +
drivers/media/platform/nxp/imx8-isi/imx8-isi-pipe.c | 1 +
3 files changed, 7 insertions(+), 2 deletions(-)
--
2.43.0
^ permalink raw reply
* [PATCH 3/3] media: imx8-isi: fix resource leaks in probe error paths and remove
From: Xiaolei Wang @ 2026-04-24 23:19 UTC (permalink / raw)
To: laurent.pinchart, mchehab, Frank.Li, s.hauer, kernel, festevam,
aisheng.dong, jacopo, guoniu.zhou, s.riedmueller, Xiaolei.Wang
Cc: linux-media, imx, linux-arm-kernel, linux-kernel, stable
In-Reply-To: <20260424231926.406079-1-xiaolei.wang@windriver.com>
mxc_isi_probe() allocates isi->pipes with kzalloc_objs() but never
frees it on any probe failure path or in mxc_isi_remove(), leaking
the allocation on every failed probe and every normal unbind.
Additionally, when mxc_isi_pipe_init() fails partway through the
channel loop or when mxc_isi_v4l2_init() fails, the already
initialized pipes are not cleaned up — their media entities and
mutexes are leaked.
Fix the pipes memory leak by switching from kzalloc_objs() to
devm_kcalloc(), which ties the allocation lifetime to the device
and eliminates the need for explicit kfree() in all error paths
and in mxc_isi_remove().
Fix the pipe init leak by cleaning up already-initialized pipes
in the err_xbar error path.
Fixes: cf21f328fcaf ("media: nxp: Add i.MX8 ISI driver")
Cc: stable@vger.kernel.org
Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com>
---
drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
index 2d639b789910..8533a979d60a 100644
--- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
+++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
@@ -485,7 +485,8 @@ static int mxc_isi_probe(struct platform_device *pdev)
isi->pdata = of_device_get_match_data(dev);
- isi->pipes = kzalloc_objs(isi->pipes[0], isi->pdata->num_channels);
+ isi->pipes = devm_kcalloc(dev, isi->pdata->num_channels,
+ sizeof(*isi->pipes), GFP_KERNEL);
if (!isi->pipes)
return -ENOMEM;
@@ -538,6 +539,8 @@ static int mxc_isi_probe(struct platform_device *pdev)
return 0;
err_xbar:
+ while (i--)
+ mxc_isi_pipe_cleanup(&isi->pipes[i]);
mxc_isi_crossbar_cleanup(&isi->crossbar);
return ret;
--
2.43.0
^ permalink raw reply related
* [PATCH 1/3] media: imx8-isi: fix use-after-free on remove
From: Xiaolei Wang @ 2026-04-24 23:19 UTC (permalink / raw)
To: laurent.pinchart, mchehab, Frank.Li, s.hauer, kernel, festevam,
aisheng.dong, jacopo, guoniu.zhou, s.riedmueller, Xiaolei.Wang
Cc: linux-media, imx, linux-arm-kernel, linux-kernel, stable
In-Reply-To: <20260424231926.406079-1-xiaolei.wang@windriver.com>
KASAN reports a slab-use-after-free in __media_entity_remove_link()
during rmmod of imx8_isi:
BUG: KASAN: slab-use-after-free in __media_entity_remove_link+0x608/0x650
Read of size 2 at addr ffff0000d47cb02a by task rmmod/724
Call trace:
__media_entity_remove_link+0x608/0x650
__media_entity_remove_links+0x78/0x144
__media_device_unregister_entity+0x150/0x280
media_device_unregister_entity+0x48/0x68
v4l2_device_unregister_subdev+0x158/0x300
v4l2_async_unbind_subdev_one+0x22c/0x358
v4l2_async_nf_unbind_all_subdevs+0xfc/0x1c0
v4l2_async_nf_unregister+0x5c/0x14c
mxc_isi_remove+0x124/0x2a0 [imx8_isi]
Allocated by task 249:
__kmalloc_noprof+0x27c/0x690
mxc_isi_crossbar_init+0x22c/0x560 [imx8_isi]
Freed by task 724:
kfree+0x1e4/0x5b0
mxc_isi_crossbar_cleanup+0x34/0x80 [imx8_isi]
mxc_isi_remove+0x11c/0x2a0 [imx8_isi]
The problem is that mxc_isi_remove() calls mxc_isi_crossbar_cleanup()
before mxc_isi_v4l2_cleanup(). The crossbar cleanup frees the media
entity pads, but the subsequent v4l2 cleanup still tries to remove
media links that reference those pads.
Fix this by calling mxc_isi_v4l2_cleanup() before
mxc_isi_crossbar_cleanup() to ensure all media entities are properly
unregistered while the pads are still valid.
Fixes: cf21f328fcaf ("media: nxp: Add i.MX8 ISI driver")
Cc: stable@vger.kernel.org
Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com>
---
drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
index 4bf8570e1b9e..2d639b789910 100644
--- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
+++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
@@ -556,8 +556,8 @@ static void mxc_isi_remove(struct platform_device *pdev)
mxc_isi_pipe_cleanup(pipe);
}
- mxc_isi_crossbar_cleanup(&isi->crossbar);
mxc_isi_v4l2_cleanup(isi);
+ mxc_isi_crossbar_cleanup(&isi->crossbar);
}
static const struct of_device_id mxc_isi_of_match[] = {
--
2.43.0
^ permalink raw reply related
* [PATCH 2/3] media: imx8-isi: add missing v4l2_subdev_cleanup() in crossbar and pipe
From: Xiaolei Wang @ 2026-04-24 23:19 UTC (permalink / raw)
To: laurent.pinchart, mchehab, Frank.Li, s.hauer, kernel, festevam,
aisheng.dong, jacopo, guoniu.zhou, s.riedmueller, Xiaolei.Wang
Cc: linux-media, imx, linux-arm-kernel, linux-kernel, stable
In-Reply-To: <20260424231926.406079-1-xiaolei.wang@windriver.com>
Both mxc_isi_crossbar_init() and mxc_isi_pipe_init() call
v4l2_subdev_init_finalize() which allocates the subdev active state,
but neither mxc_isi_crossbar_cleanup() nor mxc_isi_pipe_cleanup()
calls v4l2_subdev_cleanup() to free it.
This causes a memory leak on every rmmod, reported by kmemleak:
unreferenced object 0xffff0000d06fc800 (size 192):
comm "(udev-worker)", pid 254, jiffies 4294913455
backtrace (crc 36eeae58):
kmemleak_alloc+0x34/0x40
__kvmalloc_node_noprof+0x5f8/0x7d8
__v4l2_subdev_state_alloc+0x1fc/0x30c
__v4l2_subdev_init_finalize+0x178/0x368
Add the missing v4l2_subdev_cleanup() calls before media_entity_cleanup()
in both crossbar and pipe cleanup paths.
Fixes: cf21f328fcaf ("media: nxp: Add i.MX8 ISI driver")
Cc: stable@vger.kernel.org
Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com>
---
drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c | 1 +
drivers/media/platform/nxp/imx8-isi/imx8-isi-pipe.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
index 605a45124103..c580c831972e 100644
--- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
+++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
@@ -491,6 +491,7 @@ int mxc_isi_crossbar_init(struct mxc_isi_dev *isi)
void mxc_isi_crossbar_cleanup(struct mxc_isi_crossbar *xbar)
{
+ v4l2_subdev_cleanup(&xbar->sd);
media_entity_cleanup(&xbar->sd.entity);
kfree(xbar->pads);
kfree(xbar->inputs);
diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-pipe.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-pipe.c
index a41c51dd9ce0..cb50af2270f6 100644
--- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-pipe.c
+++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-pipe.c
@@ -819,6 +819,7 @@ void mxc_isi_pipe_cleanup(struct mxc_isi_pipe *pipe)
{
struct v4l2_subdev *sd = &pipe->sd;
+ v4l2_subdev_cleanup(sd);
media_entity_cleanup(&sd->entity);
mutex_destroy(&pipe->lock);
}
--
2.43.0
^ permalink raw reply related
* Re: [PATCH RFC bpf-next 0/8] bpf: add support for KASAN checks in JITed programs
From: Alexei Starovoitov @ 2026-04-24 23:28 UTC (permalink / raw)
To: Ihor Solodrai
Cc: Alexis Lothoré (eBPF Foundation), Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau,
Eduard Zingerman, Kumar Kartikeya Dwivedi, Song Liu,
Yonghong Song, Jiri Olsa, John Fastabend, David S. Miller,
David Ahern, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, X86 ML, H. Peter Anvin, Shuah Khan, Maxime Coquelin,
Alexandre Torgue, Andrey Ryabinin, Alexander Potapenko,
Andrey Konovalov, Dmitry Vyukov, Vincenzo Frascino, Andrew Morton,
ebpf, Bastien Curutchet, Thomas Petazzoni, Xu Kuohai, bpf, LKML,
Network Development, open list:KERNEL SELFTEST FRAMEWORK,
linux-stm32, linux-arm-kernel, kasan-dev, linux-mm
In-Reply-To: <71fb19ff-6dde-43f4-a0e9-5c8cf2ba4ed4@linux.dev>
On Fri, Apr 24, 2026 at 4:10 PM Ihor Solodrai <ihor.solodrai@linux.dev> wrote:
>
> I wonder if it's feasible to implement KASAN support on the verifier
> side in post-verification fixups. AI slop for illustration:
>
> ;; Original (1 BPF insn):
> dst = *(u64 *)(src + off) ; BPF_LDX | BPF_MEM | BPF_DW
>
> ;; Rewrite (~7 BPF insns):
> r_tmp1 = src ; BPF_MOV64_REG
> r_tmp1 += off ; BPF_ALU64 | BPF_ADD | K (full address)
> r_tmp2 = r_tmp1 ; copy
> r_tmp2 >>= 3 ; KASAN_SHADOW_SCALE_SHIFT
> r_tmp2 += KASAN_SHADOW_OFFSET ; shadow address
> r_tmp3 = *(u8 *)(r_tmp2 + 0) ; BPF_LDX | BPF_B (load shadow byte)
> if r_tmp3 != 0 goto +2 ; BPF_JNE | PC+2
> dst = *(u64 *)(src + off) ; original access (fast path)
> goto +1 ; skip slowpath
> call __asan_report_load8 ; BPF kfunc
> dst = *(u64 *)(src + off) ; retry the access after report (non-fatal)
>
> A sort of inline kasan directly in BPF.
>
> There are plenty of issues with it: instruction limit, exposing asan
> API as kfuncs, etc. On the flip side we get cross-arch support out of
> the box with no or mininal JIT changes.
>
> Honestly I'm not excited about this approach, but curious if anyone
> thought about this, or maybe it was already discussed?
We discussed this.
It won't work because we don't have that many temp registers for once
and second it has to preserve all (both callee and caller saved regs).
This is arch specific.
Second, we do not want other archs. This feature is x86-64 only.
It's being added to find _verifier_ bugs. To do that one arch is enough.
>
> > - not all memory accessing BPF instructions are being instrumented:
> > - it focuses on STX/LDX instructions
> > - it discards instructions accessing BPF program stack (already
> > monitored by page guards)
> > - it discards possibly faulting instructions, like BPF_PROBE_MEM or
> > BPF_PROBE_ATOMIC insns
> >
> > The series is marked and sent as RFC:
> > - to allow collecting feedback early and make sure that it goes into the
> > right direction
> > - because it depends on Xu's work to pass data between the verifier and
> > JIT compilers. This work is not merged yet, see [2]. I have been
> > tracking the various revisions he sent on the ML and based my local
> > branch on his work
> > - because tests brought by this series currently can't run on BPF CI:
> > they expect kasan multishot to be enabled, otherwise the first test
> > will make all other kasan-related tests fail.
>
> AFAICT this can be trivially fixed on BPF CI side, we just need to set
> kasan_multi_shot for the VMs running the tests. I will do that, your
> next revision doesn't have to be and RFC.
+1
> > - because some cases like atomic loads/stores are not instrumented yet
> > (and are still making me scratch my head)
> > - because it will hopefully provide a good basis to discuss the topic at
> > LSFMMBPF (see [3])
>
> Apparently, KASAN reporting routine takes a lock [1]:
>
> __asan_load()
> -> check_region_inline()
> -> kasan_report()
> -> start_report()
> -> raw_spin_lock_irqsave(&report_lock, *flags);
>
> BPF programs can run in NMI context, and so it appears to be possible
> to get an unflagged (because of lockdep_off() in start_report)
> deadlock, if an NMI fires on a CPU already holding report_lock.
> Although I guess you'd need two KASAN bugs to happen
> simultaneously for that to occur?... A rare event, I would hope.
>
> It could be addressed with either in_nmi() check at runtime, or
> forbidding kasan for NMI-runnable BPF program types.
We don't need that. If this bpf KASAN finds a bug, it means that
it found a verifier bug. All things are out of the window.
kasan_report() splat can just as well be the last thing that users will see.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox