[GIT pull] core/urgent for v5.18-rc6

[GIT pull] core/urgent for v5.18-rc6

[Thomas Gleixner]

Linus,

please pull the latest core/urgent branch from:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git core-urgent-2022-05-08

up to:  2667ed10d9f0: mm: Fix PASID use-after-free issue


A single bugfix for the PASID management code, which freed the PASID too
early. The PASID needs to be tied to the mm lifetime, not to the address
space lifetime.

Thanks,

	tglx

------------------>
Fenghua Yu (1):
      mm: Fix PASID use-after-free issue


 kernel/fork.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/fork.c b/kernel/fork.c
index 9796897560ab..35a3beff140b 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -792,6 +792,7 @@ void __mmdrop(struct mm_struct *mm)
 	mmu_notifier_subscriptions_destroy(mm);
 	check_mm(mm);
 	put_user_ns(mm->user_ns);
+	mm_pasid_drop(mm);
 	free_mm(mm);
 }
 EXPORT_SYMBOL_GPL(__mmdrop);
@@ -1190,7 +1191,6 @@ static inline void __mmput(struct mm_struct *mm)
 	}
 	if (mm->binfmt)
 		module_put(mm->binfmt->module);
-	mm_pasid_drop(mm);
 	mmdrop(mm);
 }
 

[GIT pull] irq/urgent for v5.18-rc6

[Thomas Gleixner]

Linus,

please pull the latest irq/urgent branch from:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git irq-urgent-2022-05-08

up to:  8707898e22fd: genirq: Synchronize interrupt thread startup


A fix for the threaded interrupt core. A quick sequence of
request/free_irq() can result in a hang because the interrupt thread did
not reach the thread function and got stopped in the kthread core
already. That leaves a state active counter arround which makes a
invocation of synchronized_irq() on that interrupt hang forever. Ensure
that the thread reached the thread function in request_irq() to prevent
that.

Thanks,

	tglx

------------------>
Thomas Pfaff (1):
      genirq: Synchronize interrupt thread startup


 kernel/irq/internals.h |  2 ++
 kernel/irq/irqdesc.c   |  2 ++
 kernel/irq/manage.c    | 39 +++++++++++++++++++++++++++++----------
 3 files changed, 33 insertions(+), 10 deletions(-)

diff --git a/kernel/irq/internals.h b/kernel/irq/internals.h
index 99cbdf55a8bd..f09c60393e55 100644
--- a/kernel/irq/internals.h
+++ b/kernel/irq/internals.h
@@ -29,12 +29,14 @@ extern struct irqaction chained_action;
  * IRQTF_WARNED    - warning "IRQ_WAKE_THREAD w/o thread_fn" has been printed
  * IRQTF_AFFINITY  - irq thread is requested to adjust affinity
  * IRQTF_FORCED_THREAD  - irq action is force threaded
+ * IRQTF_READY     - signals that irq thread is ready
  */
 enum {
 	IRQTF_RUNTHREAD,
 	IRQTF_WARNED,
 	IRQTF_AFFINITY,
 	IRQTF_FORCED_THREAD,
+	IRQTF_READY,
 };
 
 /*
diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
index 939d21cd55c3..0099b87dd853 100644
--- a/kernel/irq/irqdesc.c
+++ b/kernel/irq/irqdesc.c
@@ -407,6 +407,7 @@ static struct irq_desc *alloc_desc(int irq, int node, unsigned int flags,
 	lockdep_set_class(&desc->lock, &irq_desc_lock_class);
 	mutex_init(&desc->request_mutex);
 	init_rcu_head(&desc->rcu);
+	init_waitqueue_head(&desc->wait_for_threads);
 
 	desc_set_defaults(irq, desc, node, affinity, owner);
 	irqd_set(&desc->irq_data, flags);
@@ -575,6 +576,7 @@ int __init early_irq_init(void)
 		raw_spin_lock_init(&desc[i].lock);
 		lockdep_set_class(&desc[i].lock, &irq_desc_lock_class);
 		mutex_init(&desc[i].request_mutex);
+		init_waitqueue_head(&desc[i].wait_for_threads);
 		desc_set_defaults(i, &desc[i], node, NULL, NULL);
 	}
 	return arch_early_irq_init();
diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index c03f71d5ec10..e3e245a4fd70 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -1248,6 +1248,31 @@ static void irq_wake_secondary(struct irq_desc *desc, struct irqaction *action)
 	raw_spin_unlock_irq(&desc->lock);
 }
 
+/*
+ * Internal function to notify that a interrupt thread is ready.
+ */
+static void irq_thread_set_ready(struct irq_desc *desc,
+				 struct irqaction *action)
+{
+	set_bit(IRQTF_READY, &action->thread_flags);
+	wake_up(&desc->wait_for_threads);
+}
+
+/*
+ * Internal function to wake up a interrupt thread and wait until it is
+ * ready.
+ */
+static void wake_up_and_wait_for_irq_thread_ready(struct irq_desc *desc,
+						  struct irqaction *action)
+{
+	if (!action || !action->thread)
+		return;
+
+	wake_up_process(action->thread);
+	wait_event(desc->wait_for_threads,
+		   test_bit(IRQTF_READY, &action->thread_flags));
+}
+
 /*
  * Interrupt handler thread
  */
@@ -1259,6 +1284,8 @@ static int irq_thread(void *data)
 	irqreturn_t (*handler_fn)(struct irq_desc *desc,
 			struct irqaction *action);
 
+	irq_thread_set_ready(desc, action);
+
 	sched_set_fifo(current);
 
 	if (force_irqthreads() && test_bit(IRQTF_FORCED_THREAD,
@@ -1683,8 +1710,6 @@ __setup_irq(unsigned int irq, struct irq_desc *desc, struct irqaction *new)
 	}
 
 	if (!shared) {
-		init_waitqueue_head(&desc->wait_for_threads);
-
 		/* Setup the type (level, edge polarity) if configured: */
 		if (new->flags & IRQF_TRIGGER_MASK) {
 			ret = __irq_set_trigger(desc,
@@ -1780,14 +1805,8 @@ __setup_irq(unsigned int irq, struct irq_desc *desc, struct irqaction *new)
 
 	irq_setup_timings(desc, new);
 
-	/*
-	 * Strictly no need to wake it up, but hung_task complains
-	 * when no hard interrupt wakes the thread up.
-	 */
-	if (new->thread)
-		wake_up_process(new->thread);
-	if (new->secondary)
-		wake_up_process(new->secondary->thread);
+	wake_up_and_wait_for_irq_thread_ready(desc, new);
+	wake_up_and_wait_for_irq_thread_ready(desc, new->secondary);
 
 	register_irq_proc(irq, desc);
 	new->dir = NULL;

Re: [GIT pull] irq/urgent for v5.18-rc6

[pr-tracker-bot]

The pull request you sent on Sun,  8 May 2022 14:05:21 +0200 (CEST):

> git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git irq-urgent-2022-05-08

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/9692df0581eae29abcc925ca3c12babc6c194741

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html

[GIT pull] locking/urgent for v5.18-rc6

[Thomas Gleixner]

Linus,

please pull the latest locking/urgent branch from:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git locking-urgent-2022-05-08

up to:  60cc5468daae: futex: MAINTAINERS, .mailmap: Update André's email address


Just a email address update for MAINTAINERS and mailmap.

Thanks,

	tglx

------------------>
André Almeida (1):
      futex: MAINTAINERS, .mailmap: Update André's email address


 .mailmap    | 1 +
 MAINTAINERS | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/.mailmap b/.mailmap
index 93458154ce7d..ea1ba4a9a77e 100644
--- a/.mailmap
+++ b/.mailmap
@@ -45,6 +45,7 @@ Andrey Konovalov <andreyknvl@gmail.com> <andreyknvl@google.com>
 Andrey Ryabinin <ryabinin.a.a@gmail.com> <a.ryabinin@samsung.com>
 Andrey Ryabinin <ryabinin.a.a@gmail.com> <aryabinin@virtuozzo.com>
 Andrzej Hajda <andrzej.hajda@intel.com> <a.hajda@samsung.com>
+André Almeida <andrealmeid@igalia.com> <andrealmeid@collabora.com>
 Andy Adamson <andros@citi.umich.edu>
 Antoine Tenart <atenart@kernel.org> <antoine.tenart@bootlin.com>
 Antoine Tenart <atenart@kernel.org> <antoine.tenart@free-electrons.com>
diff --git a/MAINTAINERS b/MAINTAINERS
index 40fa1955ca3f..35dea3d12981 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8109,7 +8109,7 @@ M:	Ingo Molnar <mingo@redhat.com>
 R:	Peter Zijlstra <peterz@infradead.org>
 R:	Darren Hart <dvhart@infradead.org>
 R:	Davidlohr Bueso <dave@stgolabs.net>
-R:	André Almeida <andrealmeid@collabora.com>
+R:	André Almeida <andrealmeid@igalia.com>
 L:	linux-kernel@vger.kernel.org
 S:	Maintained
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git locking/core

Re: [GIT pull] locking/urgent for v5.18-rc6

[pr-tracker-bot]

The pull request you sent on Sun,  8 May 2022 14:05:22 +0200 (CEST):

> git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git locking-urgent-2022-05-08

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/ede4c6d78a32f9b7dfdb12d5edf8c8c4c84c729a

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html

[GIT pull] timers/urgent for v5.18-rc6

[Thomas Gleixner]

Linus,

please pull the latest timers/urgent branch from:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git timers-urgent-2022-05-08

up to:  2c33d775ef4c: timekeeping: Mark NMI safe time accessors as notrace


A fix and an email address update:

  - Mark the NMI safe time accessors notrace to prevent tracer recursion
    when they are selected as trace clocks.

  - John Stultz has a new email address

Thanks,

	tglx

------------------>
John Stultz (1):
      MAINTAINERS: Update email address for John Stultz

Kurt Kanzenbach (1):
      timekeeping: Mark NMI safe time accessors as notrace


 MAINTAINERS               | 8 ++++----
 kernel/time/timekeeping.c | 4 ++--
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 40fa1955ca3f..c2e1b7202449 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5914,7 +5914,7 @@ R:	Benjamin Gaignard <benjamin.gaignard@collabora.com>
 R:	Liam Mark <lmark@codeaurora.org>
 R:	Laura Abbott <labbott@redhat.com>
 R:	Brian Starkey <Brian.Starkey@arm.com>
-R:	John Stultz <john.stultz@linaro.org>
+R:	John Stultz <jstultz@google.com>
 L:	linux-media@vger.kernel.org
 L:	dri-devel@lists.freedesktop.org
 L:	linaro-mm-sig@lists.linaro.org (moderated for non-subscribers)
@@ -6584,7 +6584,7 @@ F:	drivers/gpu/drm/gma500/
 DRM DRIVERS FOR HISILICON
 M:	Xinliang Liu <xinliang.liu@linaro.org>
 M:	Tian Tao  <tiantao6@hisilicon.com>
-R:	John Stultz <john.stultz@linaro.org>
+R:	John Stultz <jstultz@google.com>
 R:	Xinwei Kong <kong.kongxinwei@hisilicon.com>
 R:	Chen Feng <puck.chen@hisilicon.com>
 L:	dri-devel@lists.freedesktop.org
@@ -8845,7 +8845,7 @@ F:	Documentation/devicetree/bindings/net/hisilicon*.txt
 F:	drivers/net/ethernet/hisilicon/
 
 HIKEY960 ONBOARD USB GPIO HUB DRIVER
-M:	John Stultz <john.stultz@linaro.org>
+M:	John Stultz <jstultz@google.com>
 L:	linux-kernel@vger.kernel.org
 S:	Maintained
 F:	drivers/misc/hisi_hikey_usb.c
@@ -19784,7 +19784,7 @@ F:	drivers/net/wireless/ti/
 F:	include/linux/wl12xx.h
 
 TIMEKEEPING, CLOCKSOURCE CORE, NTP, ALARMTIMER
-M:	John Stultz <john.stultz@linaro.org>
+M:	John Stultz <jstultz@google.com>
 M:	Thomas Gleixner <tglx@linutronix.de>
 R:	Stephen Boyd <sboyd@kernel.org>
 L:	linux-kernel@vger.kernel.org
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index dcdcb85121e4..3b1398fbddaf 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -482,7 +482,7 @@ static __always_inline u64 __ktime_get_fast_ns(struct tk_fast *tkf)
  * of the following timestamps. Callers need to be aware of that and
  * deal with it.
  */
-u64 ktime_get_mono_fast_ns(void)
+u64 notrace ktime_get_mono_fast_ns(void)
 {
 	return __ktime_get_fast_ns(&tk_fast_mono);
 }
@@ -494,7 +494,7 @@ EXPORT_SYMBOL_GPL(ktime_get_mono_fast_ns);
  * Contrary to ktime_get_mono_fast_ns() this is always correct because the
  * conversion factor is not affected by NTP/PTP correction.
  */
-u64 ktime_get_raw_fast_ns(void)
+u64 notrace ktime_get_raw_fast_ns(void)
 {
 	return __ktime_get_fast_ns(&tk_fast_raw);
 }

Re: [GIT pull] timers/urgent for v5.18-rc6

[pr-tracker-bot]

The pull request you sent on Sun,  8 May 2022 14:05:24 +0200 (CEST):

> git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git timers-urgent-2022-05-08

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/ea82593bad9a77f6f14c9701c13ff7368b22f027

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html

[GIT pull] x86/urgent for v5.18-rc6

[Thomas Gleixner]

Linus,

please pull the latest x86/urgent branch from:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86-urgent-2022-05-08

up to:  59f5ede3bc0f: x86/fpu: Prevent FPU state corruption


A fix and an email address update:

 - Prevent FPU state corruption. The condition in irq_fpu_usable() grants
   FPU usage when the FPU is not used in the kernel. That's just wrong as
   it does not take the fpregs_lock()'ed regions into account. If FPU usage
   happens within such a region from interrupt context, then the FPU state
   gets corrupted. That's a long standing bug, which got unearthed by the
   recent changes to the random code.

 - Josh wants to use his kernel.org email address


Thanks,

	tglx

------------------>
Josh Poimboeuf (1):
      MAINTAINERS: Update Josh Poimboeuf's email address

Thomas Gleixner (1):
      x86/fpu: Prevent FPU state corruption


 MAINTAINERS                | 10 +++----
 arch/x86/kernel/fpu/core.c | 67 ++++++++++++++++++----------------------------
 2 files changed, 31 insertions(+), 46 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index edc96cdb85e8..1e1a2264792d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7499,7 +7499,7 @@ F:	Documentation/hwmon/f71805f.rst
 F:	drivers/hwmon/f71805f.c
 
 FADDR2LINE
-M:	Josh Poimboeuf <jpoimboe@redhat.com>
+M:	Josh Poimboeuf <jpoimboe@kernel.org>
 S:	Maintained
 F:	scripts/faddr2line
 
@@ -11348,7 +11348,7 @@ F:	drivers/mmc/host/litex_mmc.c
 N:	litex
 
 LIVE PATCHING
-M:	Josh Poimboeuf <jpoimboe@redhat.com>
+M:	Josh Poimboeuf <jpoimboe@kernel.org>
 M:	Jiri Kosina <jikos@kernel.org>
 M:	Miroslav Benes <mbenes@suse.cz>
 M:	Petr Mladek <pmladek@suse.com>
@@ -14224,7 +14224,7 @@ F:	lib/objagg.c
 F:	lib/test_objagg.c
 
 OBJTOOL
-M:	Josh Poimboeuf <jpoimboe@redhat.com>
+M:	Josh Poimboeuf <jpoimboe@kernel.org>
 M:	Peter Zijlstra <peterz@infradead.org>
 S:	Supported
 F:	tools/objtool/
@@ -18792,7 +18792,7 @@ F:	include/dt-bindings/reset/starfive-jh7100.h
 
 STATIC BRANCH/CALL
 M:	Peter Zijlstra <peterz@infradead.org>
-M:	Josh Poimboeuf <jpoimboe@redhat.com>
+M:	Josh Poimboeuf <jpoimboe@kernel.org>
 M:	Jason Baron <jbaron@akamai.com>
 R:	Steven Rostedt <rostedt@goodmis.org>
 R:	Ard Biesheuvel <ardb@kernel.org>
@@ -21444,7 +21444,7 @@ F:	arch/x86/kernel/apic/x2apic_uv_x.c
 F:	arch/x86/platform/uv/
 
 X86 STACK UNWINDING
-M:	Josh Poimboeuf <jpoimboe@redhat.com>
+M:	Josh Poimboeuf <jpoimboe@kernel.org>
 M:	Peter Zijlstra <peterz@infradead.org>
 S:	Supported
 F:	arch/x86/include/asm/unwind*.h
diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
index c049561f373a..e28ab0ecc537 100644
--- a/arch/x86/kernel/fpu/core.c
+++ b/arch/x86/kernel/fpu/core.c
@@ -41,17 +41,7 @@ struct fpu_state_config fpu_user_cfg __ro_after_init;
  */
 struct fpstate init_fpstate __ro_after_init;
 
-/*
- * Track whether the kernel is using the FPU state
- * currently.
- *
- * This flag is used:
- *
- *   - by IRQ context code to potentially use the FPU
- *     if it's unused.
- *
- *   - to debug kernel_fpu_begin()/end() correctness
- */
+/* Track in-kernel FPU usage */
 static DEFINE_PER_CPU(bool, in_kernel_fpu);
 
 /*
@@ -59,42 +49,37 @@ static DEFINE_PER_CPU(bool, in_kernel_fpu);
  */
 DEFINE_PER_CPU(struct fpu *, fpu_fpregs_owner_ctx);
 
-static bool kernel_fpu_disabled(void)
-{
-	return this_cpu_read(in_kernel_fpu);
-}
-
-static bool interrupted_kernel_fpu_idle(void)
-{
-	return !kernel_fpu_disabled();
-}
-
-/*
- * Were we in user mode (or vm86 mode) when we were
- * interrupted?
- *
- * Doing kernel_fpu_begin/end() is ok if we are running
- * in an interrupt context from user mode - we'll just
- * save the FPU state as required.
- */
-static bool interrupted_user_mode(void)
-{
-	struct pt_regs *regs = get_irq_regs();
-	return regs && user_mode(regs);
-}
-
 /*
  * Can we use the FPU in kernel mode with the
  * whole "kernel_fpu_begin/end()" sequence?
- *
- * It's always ok in process context (ie "not interrupt")
- * but it is sometimes ok even from an irq.
  */
 bool irq_fpu_usable(void)
 {
-	return !in_interrupt() ||
-		interrupted_user_mode() ||
-		interrupted_kernel_fpu_idle();
+	if (WARN_ON_ONCE(in_nmi()))
+		return false;
+
+	/* In kernel FPU usage already active? */
+	if (this_cpu_read(in_kernel_fpu))
+		return false;
+
+	/*
+	 * When not in NMI or hard interrupt context, FPU can be used in:
+	 *
+	 * - Task context except from within fpregs_lock()'ed critical
+	 *   regions.
+	 *
+	 * - Soft interrupt processing context which cannot happen
+	 *   while in a fpregs_lock()'ed critical region.
+	 */
+	if (!in_hardirq())
+		return true;
+
+	/*
+	 * In hard interrupt context it's safe when soft interrupts
+	 * are enabled, which means the interrupt did not hit in
+	 * a fpregs_lock()'ed critical region.
+	 */
+	return !softirq_count();
 }
 EXPORT_SYMBOL(irq_fpu_usable);
 

Re: [GIT pull] x86/urgent for v5.18-rc6

[pr-tracker-bot]

The pull request you sent on Sun,  8 May 2022 14:07:12 +0200 (CEST):

> git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86-urgent-2022-05-08

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/27b5d61c0c0c4ba347b514ade27f7919b032778e

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html

Re: [GIT pull] core/urgent for v5.18-rc6

[Linus Torvalds]

On Sun, May 8, 2022 at 5:05 AM Thomas Gleixner <tglx@linutronix.de> wrote:
>
> A single bugfix for the PASID management code, which freed the PASID too
> early. The PASID needs to be tied to the mm lifetime, not to the address
> space lifetime.

So I have to once more complain about the -tip tree "Link:" usage.

Again, the commit has a link to the patch *submission*, which is
almost entirely useless. There's no link to the actual problem the
patch fixes.

It does have a "Fixes:" link, and it has an "explanation", but the
explanation doesn't actually make much sense to me.

The *sensible* thing for an iommu driver to do is to just do a
mmget/mmput, and then the whole "drop PASIX on last mmput() (ie in
__mmput)" makes lots of sense.

The commit claims that's not what the iommu drivers do, but it's
really hard to tell *what* it is that does a mmgrab. It just says

  "the IOMMU driver holds a reference on the mm itself, not the address space"

but then I tried to see where such references are held, and I couldn't find it.

*Must* users do mmget/mmput, and the commit even says that's the
logical thing to do. Apparently something that the iommu code relies
on does the mmgrab/mmdrop thing, but I really would have liked to know
what that is.

Yes, "mmgab" is the right thing to do if all you want is the "struct
mm_struct", and don't actually care about the page tables themselves,
and don't want to have the refcount keep page tables alive. But you'd
think the iommu code really does want the page tables.

So it would have been really nice to have an explanation for what it
was that did the mmgrab, especially since the commit itself makes it
clear that the logical thing to do seems to be just mmget/put. No?

I _really_ wish the -tip tree had more links to the actual problem
reports and explanations, rather than links to the patch submission.

One has the actual issue. The other just has the information that is
already in the commit, and the "Link:" adds almost no actual value
(yes, yes, you can see late "Acked-by" etc, but that really isn't
worth it).

I've pulled this, because I definitely believe the issue is real. I
just wish I could see _what_ the issue was.

Put another way: I can see that

    Reported-by: Zhangfei Gao <zhangfei.gao@foxmail.com>

in the commit, but I don't have a clue what the actual report was, and
there really isn't enough information in the commit itself, except for
a fairly handwavy "Device drivers might, for instance, still need to
flush operations.."

I don't want to know what device drivers _might_ do. I would want to
have an actual pointer to what they do and where.

I suspect it's related to mmu_notifiers or something, but I really
would have liked a more exact "this is where things go wrong".

I also *suspect* that this is all about that _loong_ thread (picking
one email almost at random) here:

   https://lore.kernel.org/all/a139dbad-2f42-913b-677c-ef35f1eebfed@intel.com/

but if so, the confusion I see in that thread is even more reason to
have more background for what is going on.

Anyway, this is merged in my tree, but I'm having trouble following
the logic, which is why I'm writing this long email to complain about
lack of context.

               Linus

Re: [GIT pull] core/urgent for v5.18-rc6

[Linus Torvalds]

On Sun, May 8, 2022 at 11:00 AM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> I suspect it's related to mmu_notifiers or something, but I really
> would have liked a more exact "this is where things go wrong".

Looks like it is

 ->(*sva_bind)()
    -> intel_svm_bind_mm()
      -> mmu_notifier_register(&svm->notifier, mm)

and yes, the mmu notifiers annoyingly end up doing an mmgrab, because
they actually don't want to affect the state of mm teardown, they just
want to be notified about it.

Annoying, but it seems to explain the issue if I followed this right.

Unlike the commit message.

                Linus

Re: [GIT pull] core/urgent for v5.18-rc6

[Linus Torvalds]

On Sun, May 8, 2022 at 11:09 AM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> Looks like it is
>
>  ->(*sva_bind)()
>     -> intel_svm_bind_mm()
>       -> mmu_notifier_register(&svm->notifier, mm)
>
> and yes, the mmu notifiers annoyingly end up doing an mmgrab [..]

Side note: quite independently of this mmgrab issue, I think the code
in question is *very* suspect and horrendously fragile.

In particular, the code ends up being called through things like this:

     handle = iommu_sva_bind_device(uacce->parent, current->mm, NULL);

and then that Intel svm.c code does this:

                svm->pasid = mm->pasid;
                svm->mm = mm;
                svm->flags = flags;

and saves off that mm pointer in the 'svm' structure.

AND IT NEVER TAKES ANY REFERENCE TO IT AT ALL!

It then does

        mm = svm->mm;

later at some unspecified time, and the 'mm' might long since have died.

In other words, the code works almost by accident - the only user of
the 'mm' pointer seems to be that mmu_notifier_register() thing, so it
basically treats the 'struct mm_struct' as something as a random
cookie.

And yes, the mmu notifiers do then take that mmgrab reference to the
mm, so it all works.

But it sure looks horrendously ugly. Saving off a 'struct mm_struct'
pointer with having basically an accidental reference to it is WRONG.

In fact, it will save off that pointer whether it then actually does
the mmu_notifier thing on it, because the code actually does

                [...]
                svm->mm = mm;
                svm->flags = flags;
                INIT_LIST_HEAD_RCU(&svm->devs);

                if (!(flags & SVM_FLAG_SUPERVISOR_MODE)) {
                        svm->notifier.ops = &intel_mmuops;
                        ret = mmu_notifier_register(&svm->notifier, mm);
                [...]

so that mmu_notifier_register() call is conditional. On the freeing
path, it then uses that "svm->notifier.ops" pointer as a "did we
register this thing or not" flag, so again - it all technically
*works*, but this is all horrendously ugly and wrong on so many
levels, keeping pointers around with very dubious reference counting
indeed.

               Linus

Re: [GIT pull] core/urgent for v5.18-rc6

[Jean-Philippe Brucker]

Hi,

On Sun, May 08, 2022 at 11:36:23AM -0700, Linus Torvalds wrote:
> And yes, the mmu notifiers do then take that mmgrab reference to the
> mm, so it all works.
> 
> But it sure looks horrendously ugly. Saving off a 'struct mm_struct'
> pointer with having basically an accidental reference to it is WRONG.

Yes it is fragile. We're currently reworking iommu_sva_bind_device() to
factor more code into the IOMMU core and to add explicit mmgrab/mmdrop, so
once this lands we won't depend on mmu_notifier_register() anymore to hold
the mm reference.

https://lore.kernel.org/linux-iommu/20220502014842.991097-9-baolu.lu@linux.intel.com/

Thanks,
Jean

Link: tag and links to submission and reports (was: Re: [GIT pull] core/urgent for v5.18-rc6)

[Thorsten Leemhuis]

On 08.05.22 20:00, Linus Torvalds wrote:
> On Sun, May 8, 2022 at 5:05 AM Thomas Gleixner <tglx@linutronix.de> wrote:
>>
>> A single bugfix for the PASID management code, which freed the PASID too
>> early. The PASID needs to be tied to the mm lifetime, not to the address
>> space lifetime.
> 
> So I have to once more complain about the -tip tree "Link:" usage.

Many thx for reminding people about the tag.  FWIW, that's a problem in
a lot or subsystems and makes my regression tracking efforts hard, as my
tracking bot relies on the 'Link:' tag. If it's missing I thus have to
manually search if patches were posted or committed to fix a regression,
which makes the tracking hard and annoying. :-/

> Again, the commit has a link to the patch *submission*, which is
> almost entirely useless. There's no link to the actual problem the
> patch fixes.

It seems quite a few developers are under the impressions that "Link:"
is just for the patch submission and not to be used for other things.
That's why some people invented other tags. I see "BugLink" quite often
these days, but there are also other tags in use (some drm people used
"References:" for a while).

Do we care? Should we try to clean this up while making things a bit
more straight forward at the same time by making it more obvious what a
link is actually about? I once suggested we use something like
* "Submitted:" or "Posted:" for the patch submission
* "Reported:" or "BugLink:" for any reports that lead to the

That would leave "Link:" ambiguous and usable for anything else (and b4
likely could be fixed easily to set a different tag; but sure, there
would be a transition period).

But there was not much feedback on the idea. Do you think I should pick
up this again? Or is this something I should bring up during this years
kernel summit?

> [...]
> Put another way: I can see that
>     Reported-by: Zhangfei Gao <zhangfei.gao@foxmail.com>

With a "Reported:" tag like mentioned above we could stop using
"Reported-by:" if we wanted to reduce the overhead (or make it
optional). But OTOH I guess it's a bad idea, as having this in there
will motivate some people to submit reports. And is good for stats reg.
syzbot and 0-day (but I guess those could be generated from proper
links, too).

BTW: Documentation/process/5.Posting.rst states '''Be careful in the
addition of tags to your patches: only Cc: is appropriate for addition
without the explicit permission of the person named.''' Is that actually
true? A lot of people seem to set "Reported-by:" without getting
explicit permission. If that is fine I'd prepare a patch to fix the docs.

Ciao, Thorsten

Re: Link: tag and links to submission and reports (was: Re: [GIT pull] core/urgent for v5.18-rc6)

[Borislav Petkov]

On Tue, May 10, 2022 at 01:27:54PM +0200, Thorsten Leemhuis wrote:
> Many thx for reminding people about the tag.  FWIW, that's a problem in
> a lot or subsystems and makes my regression tracking efforts hard, as my
> tracking bot relies on the 'Link:' tag. If it's missing I thus have to
> manually search if patches were posted or committed to fix a regression,
> which makes the tracking hard and annoying. :-/

Here's my experience with the Link thing:

So it is trivial to take the Message-ID and turn it into a link tag and
our automation does that.

- Now, it is not a problem when that link tag points to a patch which is
part of the thread which contains the initial bug report - you just go
up-thread.

- If the link tag points to a patch which is version N and it is the
version which passed all review and gets committed, it is a bit harder
to find the previous versions and find the whole discussion how it all
arrived at version N. You can search by the Subject, ofc, which, if it
hasn't been changed, will give you the previous threads. And so on ...

- The problem is when the discussion happened somewhere and the patch
got submitted separately. I can't think of a good way to automate
that so we have to pay attention and fix the link tag by hand and add
the relevant one. And I try to do that when I'm especially awake when
applying the patch.

So I think we should simply pay attention to making sure the link tags
point to the relevant discussion. And we even document that:

 "If related discussions or any other background information behind the
 change can be found on the web, add 'Link:' tags pointing to it."

so we better follow through with it.

:-)

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: Link: tag and links to submission and reports (was: Re: [GIT pull] core/urgent for v5.18-rc6)

[Michael Ellerman]

Borislav Petkov <bp@alien8.de> writes:
> On Tue, May 10, 2022 at 01:27:54PM +0200, Thorsten Leemhuis wrote:
>> Many thx for reminding people about the tag.  FWIW, that's a problem in
>> a lot or subsystems and makes my regression tracking efforts hard, as my
>> tracking bot relies on the 'Link:' tag. If it's missing I thus have to
>> manually search if patches were posted or committed to fix a regression,
>> which makes the tracking hard and annoying. :-/
>
> Here's my experience with the Link thing:
>
> So it is trivial to take the Message-ID and turn it into a link tag and
> our automation does that.
>
> - Now, it is not a problem when that link tag points to a patch which is
> part of the thread which contains the initial bug report - you just go
> up-thread.
>
> - If the link tag points to a patch which is version N and it is the
> version which passed all review and gets committed, it is a bit harder
> to find the previous versions and find the whole discussion how it all
> arrived at version N. You can search by the Subject, ofc, which, if it
> hasn't been changed, will give you the previous threads. And so on ...
>
> - The problem is when the discussion happened somewhere and the patch
> got submitted separately. I can't think of a good way to automate
> that so we have to pay attention and fix the link tag by hand and add
> the relevant one. And I try to do that when I'm especially awake when
> applying the patch.

That doesn't scale though, it puts more work on maintainers, who already
don't have enough time.

The *submitter* should be putting all the relevant info in the patch,
including any links to other discussions, previous versions etc. etc.

Then the maintainer can automatically add the "Link:" tag pointing to
the submission, and everything is there in the archive.

One advantage of linking back to the original submission is that if the
patch doesn't have all the relevant info, anyone can post replies adding
context or linking to other places, even after the patch has been
committed.

cheers

Re: Link: tag and links to submission and reports (was: Re: [GIT pull] core/urgent for v5.18-rc6)

[Theodore Ts'o]

On Wed, May 11, 2022 at 10:37:35AM +0200, Borislav Petkov wrote:
> - The problem is when the discussion happened somewhere and the patch
> got submitted separately. I can't think of a good way to automate
> that so we have to pay attention and fix the link tag by hand and add
> the relevant one. And I try to do that when I'm especially awake when
> applying the patch.

I would argue that it should be the patch submitter's responsibility
to explicitly add a URL to the problem report.  In some cases this
might be a pointer to a bug tracking system; in other cases it might
be a URL to lore.kernel.org.

					- Ted

Re: Link: tag and links to submission and reports (was: Re: [GIT pull] core/urgent for v5.18-rc6)

[Linus Torvalds]

On Wed, May 11, 2022 at 8:50 AM Theodore Ts'o <tytso@mit.edu> wrote:
>
> I would argue that it should be the patch submitter's responsibility
> to explicitly add a URL to the problem report.

I agree in the perfect case.

But in practice, we have a lot more patch submitters than we have
maintainers, and not all "leaf developers" necessarily know how to do
everything.

So the maintainer should probably expect to fix things up. Not always,
but also not a "the developer should have done this, so I won't do it"

This isn't so different from the fact that not everybody writes
English proficiently - people do hopefully end up fixing things up as
they get passed onwards.

               Linus

Re: Link: tag and links to submission and reports (was: Re: [GIT pull] core/urgent for v5.18-rc6)

[Borislav Petkov]

On Wed, May 11, 2022 at 08:55:34AM -0700, Linus Torvalds wrote:
> On Wed, May 11, 2022 at 8:50 AM Theodore Ts'o <tytso@mit.edu> wrote:
> >
> > I would argue that it should be the patch submitter's responsibility
> > to explicitly add a URL to the problem report.
> 
> I agree in the perfect case.
> 
> But in practice, we have a lot more patch submitters than we have
> maintainers, and not all "leaf developers" necessarily know how to do
> everything.
> 
> So the maintainer should probably expect to fix things up. Not always,
> but also not a "the developer should have done this, so I won't do it"
> 
> This isn't so different from the fact that not everybody writes
> English proficiently - people do hopefully end up fixing things up as
> they get passed onwards.

And, in addition, what happens most often in my experience is I
constantly get to point submitters to our process documentation -
submitting-patches especially - as not a small number of them are not
aware of different aspects of the whole patch dance: tags, SOB chains,
etc. And the Link tag is no exception here.

So yeah, the maintainer is kinda the last one to make sure the patch
looks somewhat sane and documents the whole issue so that years from
now, it can still be understood what we were fixing there.

And I guess important part of that documentation is setting the proper
Link so...

And as said, yeah, we won't be perfect and catch all cases but at least
we can pay attention.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: Link: tag and links to submission and reports (was: Re: [GIT pull] core/urgent for v5.18-rc6)

[Thorsten Leemhuis]

On 11.05.22 21:35, Borislav Petkov wrote:
> On Wed, May 11, 2022 at 08:55:34AM -0700, Linus Torvalds wrote:
>> On Wed, May 11, 2022 at 8:50 AM Theodore Ts'o <tytso@mit.edu> wrote:
>>>
>>> I would argue that it should be the patch submitter's responsibility
>>> to explicitly add a URL to the problem report.
>>
>> I agree in the perfect case.
>>
>> But in practice, we have a lot more patch submitters than we have
>> maintainers, and not all "leaf developers" necessarily know how to do
>> everything.
>>
>> So the maintainer should probably expect to fix things up. Not always,
>> but also not a "the developer should have done this, so I won't do it"
>>
>> This isn't so different from the fact that not everybody writes
>> English proficiently - people do hopefully end up fixing things up as
>> they get passed onwards.
> 
> And, in addition, what happens most often in my experience is I
> constantly get to point submitters to our process documentation -
> submitting-patches especially - as not a small number of them are not
> aware of different aspects of the whole patch dance: tags, SOB chains,
> etc. And the Link tag is no exception here.

Which leads to the question: can we (and do we want to) teach
scripts/checkpatch.pl to point out when a Link: tag is missing and
likely appropriate? If a "Reported-by:" is present there should be a
"Link:" as well, unless the issue was reported privately, via IRC or
something like that. A "Fixes:" tag is also a strong indicator that a
link might be appropriate, but not as good.

Ciao, Thorsten

Re: Link: tag and links to submission and reports (was: Re: [GIT pull] core/urgent for v5.18-rc6)

[Borislav Petkov]

On Thu, May 12, 2022 at 10:43:07AM +0200, Thorsten Leemhuis wrote:
> Which leads to the question: can we (and do we want to) teach
> scripts/checkpatch.pl to point out when a Link: tag is missing and
> likely appropriate? If a "Reported-by:" is present there should be a
> "Link:" as well, unless the issue was reported privately, via IRC or
> something like that. A "Fixes:" tag is also a strong indicator that a
> link might be appropriate, but not as good.

All good ideas, sure.

At least pointing it out as a hint - not necessarily as a warning -
would be a good idea. And say, "hey, and while you're adding a Link
tag, pls make sure it points to the mail which has the most relevant
discussion on the matter your patch is fixing."

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: Link: tag and links to submission and reports (was: Re: [GIT pull] core/urgent for v5.18-rc6)

[Theodore Ts'o]

On Wed, May 11, 2022 at 08:55:34AM -0700, Linus Torvalds wrote:
> On Wed, May 11, 2022 at 8:50 AM Theodore Ts'o <tytso@mit.edu> wrote:
> >
> > I would argue that it should be the patch submitter's responsibility
> > to explicitly add a URL to the problem report.
> 
> I agree in the perfect case.
> 
> But in practice, we have a lot more patch submitters than we have
> maintainers, and not all "leaf developers" necessarily know how to do
> everything.
> 
> So the maintainer should probably expect to fix things up. Not always,
> but also not a "the developer should have done this, so I won't do it"

Sure... but *because* maintainers are significantly outnumbered by the
patch submitters, what we should document is that it is the
developer's responsibility to provide the link, just as it is the
developer's responsibility to at least *try* to write a clear commit
description.

> This isn't so different from the fact that not everybody writes
> English proficiently - people do hopefully end up fixing things up as
> they get passed onwards.

Maintainers can be the backstop, sure, but if we are trying to make
Maintainers scale well, developers should at least feel obligated to
**try** to lighten the load on maintainers to the limits of their
ability.

And I have a bit more sympathy over someone who doesn't speak English
as their first language, over someone who can't be bothered to look up
a bug report link or a Syzkaller ID.

I guess we could always NACK such a patch until they provide that
information, but if it's for a critical bug fix, that's not
necessarily a good alternative either.  I guess that's why the
Maintainers get paid the big bucks....

					- Ted

Re: [GIT pull] core/urgent for v5.18-rc6

[pr-tracker-bot]

The pull request you sent on Sun,  8 May 2022 14:05:19 +0200 (CEST):

> git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git core-urgent-2022-05-08

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/1f8c5dff000d96b66273a0bf57dbf4d505c730cc

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html