* [PATCH] hazptr: handle NULL address in hazptr_detach
@ 2026-09-08 15:22 Mathieu Desnoyers
2026-09-08 15:24 ` Bradley Morgan
2026-09-10 2:16 ` kernel test robot
0 siblings, 2 replies; 10+ messages in thread
From: Mathieu Desnoyers @ 2026-09-08 15:22 UTC (permalink / raw)
To: Paul E . McKenney
Cc: linux-kernel, Mathieu Desnoyers, kernel test robot,
Bradley Morgan, Boqun Feng, rcu, lkmm
When hazptr_acquire loads a NULL pointer, it sets:
- slot_item->slot.addr = NULL,
- slot_item->ctx.ctx = ctx
- ctx->slot = slot
And it returns NULL.
Then hazptr_detach is called on this ctx, it will act on the ctx as if
needed to be promoted to backup slot, even though it has a NULL addr.
Looking at what hazptr_note_context_switch() does before promoting
to backup slot, it checks for a NULL slot->addr, which is exactly
what is missing from hazptr_detach.
With this in place there would be no need to explicitly check the
hazptr_acquire() return value before calling hazptr_detach().
hazptr_release() has a early return check for NULL addr as well, so it
makes sense that detach does an early return (no-op) similarly.
Fixes: 6357ec235c59 ("hazptrtorture: Fix hazptr ownership issue")
Reported-by: kernel test robot <yi1.lai@intel.com>
Closes: https://lore.kernel.org/oe-lkp/202608130915.62b53936-lkp@intel.com
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Reviewed-by: Bradley Morgan <brads@mainlining.org>
Cc: Paul E. McKenney <paulmck@kernel.org>
Cc: Boqun Feng <boqun@kernel.org>
Cc: Bradley Morgan <brads@mainlining.org>
Cc: <rcu@vger.kernel.org>
Cc: <lkmm@lists.linux.dev>
---
include/linux/hazptr.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/include/linux/hazptr.h b/include/linux/hazptr.h
index 43122c5673bd..d1670121947a 100644
--- a/include/linux/hazptr.h
+++ b/include/linux/hazptr.h
@@ -160,10 +160,12 @@ void hazptr_detach(struct hazptr_ctx *ctx)
struct hazptr_slot *slot;
guard(preempt)();
+ slot = ctx->slot;
+ if (!slot->addr)
+ return;
#ifdef CONFIG_HAZPTR_DEBUG
ctx->detach_task = ctx->detach_cpu = true;
#endif
- slot = ctx->slot;
if (unlikely(hazptr_slot_is_backup(ctx, slot)))
return;
hazptr_promote_to_backup_slot(ctx, slot);
--
2.43.0
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] hazptr: handle NULL address in hazptr_detach 2026-09-08 15:22 [PATCH] hazptr: handle NULL address in hazptr_detach Mathieu Desnoyers @ 2026-09-08 15:24 ` Bradley Morgan 2026-09-08 15:29 ` Mathieu Desnoyers 2026-09-10 2:16 ` kernel test robot 1 sibling, 1 reply; 10+ messages in thread From: Bradley Morgan @ 2026-09-08 15:24 UTC (permalink / raw) To: Mathieu Desnoyers, Paul E . McKenney Cc: linux-kernel, kernel test robot, Boqun Feng, rcu, lkmm On 8 September 2026 16:22:14 BST, Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote: >When hazptr_acquire loads a NULL pointer, it sets: > >- slot_item->slot.addr = NULL, >- slot_item->ctx.ctx = ctx >- ctx->slot = slot > >And it returns NULL. > >Then hazptr_detach is called on this ctx, it will act on the ctx as if >needed to be promoted to backup slot, even though it has a NULL addr. > >Looking at what hazptr_note_context_switch() does before promoting >to backup slot, it checks for a NULL slot->addr, which is exactly >what is missing from hazptr_detach. > >With this in place there would be no need to explicitly check the >hazptr_acquire() return value before calling hazptr_detach(). > >hazptr_release() has a early return check for NULL addr as well, so it >makes sense that detach does an early return (no-op) similarly. > You shall kill me for this!! Could you perhaps do a splat in ze commit description pls? >Fixes: 6357ec235c59 ("hazptrtorture: Fix hazptr ownership issue") >Reported-by: kernel test robot <yi1.lai@intel.com> >Closes: https://lore.kernel.org/oe-lkp/202608130915.62b53936-lkp@intel.com >Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> >Reviewed-by: Bradley Morgan <brads@mainlining.org> >Cc: Paul E. McKenney <paulmck@kernel.org> >Cc: Boqun Feng <boqun@kernel.org> >Cc: Bradley Morgan <brads@mainlining.org> >Cc: <rcu@vger.kernel.org> >Cc: <lkmm@lists.linux.dev> >--- > include/linux/hazptr.h | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > >diff --git a/include/linux/hazptr.h b/include/linux/hazptr.h >index 43122c5673bd..d1670121947a 100644 >--- a/include/linux/hazptr.h >+++ b/include/linux/hazptr.h >@@ -160,10 +160,12 @@ void hazptr_detach(struct hazptr_ctx *ctx) > struct hazptr_slot *slot; > > guard(preempt)(); >+ slot = ctx->slot; >+ if (!slot->addr) >+ return; > #ifdef CONFIG_HAZPTR_DEBUG > ctx->detach_task = ctx->detach_cpu = true; > #endif >- slot = ctx->slot; > if (unlikely(hazptr_slot_is_backup(ctx, slot))) > return; > hazptr_promote_to_backup_slot(ctx, slot); > --- Thanks! https://lore.kernel.org/all/EE579805-42F2-4C58-B752-F28779EEB717@grrlz.net/ ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] hazptr: handle NULL address in hazptr_detach 2026-09-08 15:24 ` Bradley Morgan @ 2026-09-08 15:29 ` Mathieu Desnoyers 2026-09-08 15:52 ` Bradley Morgan 2026-09-08 16:22 ` Paul E. McKenney 0 siblings, 2 replies; 10+ messages in thread From: Mathieu Desnoyers @ 2026-09-08 15:29 UTC (permalink / raw) To: Bradley Morgan, Paul E . McKenney Cc: linux-kernel, kernel test robot, Boqun Feng, rcu, lkmm On 2026-09-08 11:24, Bradley Morgan wrote: > On 8 September 2026 16:22:14 BST, Mathieu Desnoyers > <mathieu.desnoyers@efficios.com> wrote: >> When hazptr_acquire loads a NULL pointer, it sets: >> >> - slot_item->slot.addr = NULL, >> - slot_item->ctx.ctx = ctx >> - ctx->slot = slot >> >> And it returns NULL. >> >> Then hazptr_detach is called on this ctx, it will act on the ctx as if >> needed to be promoted to backup slot, even though it has a NULL addr. >> >> Looking at what hazptr_note_context_switch() does before promoting >> to backup slot, it checks for a NULL slot->addr, which is exactly >> what is missing from hazptr_detach. >> >> With this in place there would be no need to explicitly check the >> hazptr_acquire() return value before calling hazptr_detach(). >> >> hazptr_release() has a early return check for NULL addr as well, so it >> makes sense that detach does an early return (no-op) similarly. >> > > You shall kill me for this!! > > Could you perhaps do a splat in ze commit description pls? The splat is available at the "Closes" URL below. I'm not sure whether we should duplicate this verbose information ? Paul ? Thanks, Mathieu > > >> Fixes: 6357ec235c59 ("hazptrtorture: Fix hazptr ownership issue") >> Reported-by: kernel test robot <yi1.lai@intel.com> >> Closes: https://lore.kernel.org/oe-lkp/202608130915.62b53936-lkp@intel.com >> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> >> Reviewed-by: Bradley Morgan <brads@mainlining.org> >> Cc: Paul E. McKenney <paulmck@kernel.org> >> Cc: Boqun Feng <boqun@kernel.org> >> Cc: Bradley Morgan <brads@mainlining.org> >> Cc: <rcu@vger.kernel.org> >> Cc: <lkmm@lists.linux.dev> >> --- >> include/linux/hazptr.h | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/include/linux/hazptr.h b/include/linux/hazptr.h >> index 43122c5673bd..d1670121947a 100644 >> --- a/include/linux/hazptr.h >> +++ b/include/linux/hazptr.h >> @@ -160,10 +160,12 @@ void hazptr_detach(struct hazptr_ctx *ctx) >> struct hazptr_slot *slot; >> >> guard(preempt)(); >> + slot = ctx->slot; >> + if (!slot->addr) >> + return; >> #ifdef CONFIG_HAZPTR_DEBUG >> ctx->detach_task = ctx->detach_cpu = true; >> #endif >> - slot = ctx->slot; >> if (unlikely(hazptr_slot_is_backup(ctx, slot))) >> return; >> hazptr_promote_to_backup_slot(ctx, slot); >> > > --- Thanks! > https://lore.kernel.org/all/EE579805-42F2-4C58-B752-F28779EEB717@grrlz.net/ -- Mathieu Desnoyers EfficiOS Inc. https://www.efficios.com ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] hazptr: handle NULL address in hazptr_detach 2026-09-08 15:29 ` Mathieu Desnoyers @ 2026-09-08 15:52 ` Bradley Morgan 2026-09-08 16:22 ` Paul E. McKenney 1 sibling, 0 replies; 10+ messages in thread From: Bradley Morgan @ 2026-09-08 15:52 UTC (permalink / raw) To: Mathieu Desnoyers, Paul E . McKenney Cc: linux-kernel, kernel test robot, Boqun Feng, rcu, lkmm On 8 September 2026 16:29:31 BST, Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote: >On 2026-09-08 11:24, Bradley Morgan wrote: >> On 8 September 2026 16:22:14 BST, Mathieu Desnoyers >> <mathieu.desnoyers@efficios.com> wrote: >>> When hazptr_acquire loads a NULL pointer, it sets: >>> >>> - slot_item->slot.addr = NULL, >>> - slot_item->ctx.ctx = ctx >>> - ctx->slot = slot >>> >>> And it returns NULL. >>> >>> Then hazptr_detach is called on this ctx, it will act on the ctx as if >>> needed to be promoted to backup slot, even though it has a NULL addr. >>> >>> Looking at what hazptr_note_context_switch() does before promoting >>> to backup slot, it checks for a NULL slot->addr, which is exactly >>> what is missing from hazptr_detach. >>> >>> With this in place there would be no need to explicitly check the >>> hazptr_acquire() return value before calling hazptr_detach(). >>> >>> hazptr_release() has a early return check for NULL addr as well, so it >>> makes sense that detach does an early return (no-op) similarly. >>> >> >> You shall kill me for this!! >> >> Could you perhaps do a splat in ze commit description pls? > >The splat is available at the "Closes" URL below. I'm not sure whether >we should duplicate this verbose information ? > >Paul ? > This is a example of what I did. From 8861f6d5c0678a7c5089c7b272509fc5931b8437 Mon Sep 17 00:00:00 2001 From: Bradley Morgan <brads@mainlining.org> Date: Thu, 27 Aug 2026 17:43:38 +0000 Subject: ima: Check for ERR_PTR from dentry_path() in validate_hash_algo() From: Bradley Morgan <brads@mainlining.org> commit 8861f6d5c0678a7c5089c7b272509fc5931b8437 upstream. dentry_path() returns ERR_PTR(-ENAMETOOLONG) when the path exceeds the buffer. validate_hash_algo() passes the result straight to integrity_audit_msg() without checking. ERR_PTR is not NULL, so integrity_audit_message() sees a valid pointer and calls strlen() on it, which faults: BUG: unable to handle page fault for address: ffffffffffffffdc RIP: 0010:strlen+0x30/0xa0 Call Trace: audit_log_untrustedstring+0x19/0x30 integrity_audit_message+0x366/0x4f0 ima_inode_setxattr+0x512/0x5f0 Check for IS_ERR() and use NULL instead, which makes the audit message skip the name= field instead of crashing. Fixes: 4f2946aa0c45 ("IMA: introduce a new policy option func=SETXATTR_CHECK") Cc: stable@vger.kernel.org Reported-by: syzbot+5ebeb3089ea6439c37be@syzkaller.appspotmail.com Link: https://lore.kernel.org/all/6a8f89e5.1d9ded08.62e62.00bf.GAE@google.com/ Signed-off-by: Bradley Morgan <brads@mainlining.org> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> security/integrity/ima/ima_appraise.c | 2 ++ 1 file changed, 2 insertions(+) --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -754,6 +754,8 @@ static int validate_hash_algo(struct den return -EACCES; path = dentry_path(dentry, pathbuf, PATH_MAX); + if (IS_ERR(path)) + path = NULL; integrity_audit_msg(AUDIT_INTEGRITY_DATA, d_inode(dentry), path, "set_data", errmsg, -EACCES, 0); >Thanks, > >Mathieu > >> >> >>> Fixes: 6357ec235c59 ("hazptrtorture: Fix hazptr ownership issue") >>> Reported-by: kernel test robot <yi1.lai@intel.com> >>> Closes: >https://lore.kernel.org/oe-lkp/202608130915.62b53936-lkp@intel.com >>> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> >>> Reviewed-by: Bradley Morgan <brads@mainlining.org> >>> Cc: Paul E. McKenney <paulmck@kernel.org> >>> Cc: Boqun Feng <boqun@kernel.org> >>> Cc: Bradley Morgan <brads@mainlining.org> >>> Cc: <rcu@vger.kernel.org> >>> Cc: <lkmm@lists.linux.dev> >>> --- >>> include/linux/hazptr.h | 4 +++- >>> 1 file changed, 3 insertions(+), 1 deletion(-) >>> >>> diff --git a/include/linux/hazptr.h b/include/linux/hazptr.h >>> index 43122c5673bd..d1670121947a 100644 >>> --- a/include/linux/hazptr.h >>> +++ b/include/linux/hazptr.h >>> @@ -160,10 +160,12 @@ void hazptr_detach(struct hazptr_ctx *ctx) >>> struct hazptr_slot *slot; >>> >>> guard(preempt)(); >>> + slot = ctx->slot; >>> + if (!slot->addr) >>> + return; >>> #ifdef CONFIG_HAZPTR_DEBUG >>> ctx->detach_task = ctx->detach_cpu = true; >>> #endif >>> - slot = ctx->slot; >>> if (unlikely(hazptr_slot_is_backup(ctx, slot))) >>> return; >>> hazptr_promote_to_backup_slot(ctx, slot); >>> >> >> --- Thanks! >> >https://lore.kernel.org/all/EE579805-42F2-4C58-B752-F28779EEB717@grrlz.net/ > > > --- Thanks! https://lore.kernel.org/all/EE579805-42F2-4C58-B752-F28779EEB717@grrlz.net/ ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] hazptr: handle NULL address in hazptr_detach 2026-09-08 15:29 ` Mathieu Desnoyers 2026-09-08 15:52 ` Bradley Morgan @ 2026-09-08 16:22 ` Paul E. McKenney 2026-09-08 16:31 ` Bradley Morgan 2026-09-08 16:53 ` Mathieu Desnoyers 1 sibling, 2 replies; 10+ messages in thread From: Paul E. McKenney @ 2026-09-08 16:22 UTC (permalink / raw) To: Mathieu Desnoyers Cc: Bradley Morgan, linux-kernel, kernel test robot, Boqun Feng, rcu, lkmm On Tue, Sep 08, 2026 at 11:29:31AM -0400, Mathieu Desnoyers wrote: > On 2026-09-08 11:24, Bradley Morgan wrote: > > On 8 September 2026 16:22:14 BST, Mathieu Desnoyers > > <mathieu.desnoyers@efficios.com> wrote: > > > When hazptr_acquire loads a NULL pointer, it sets: > > > > > > - slot_item->slot.addr = NULL, > > > - slot_item->ctx.ctx = ctx > > > - ctx->slot = slot > > > > > > And it returns NULL. > > > > > > Then hazptr_detach is called on this ctx, it will act on the ctx as if > > > needed to be promoted to backup slot, even though it has a NULL addr. > > > > > > Looking at what hazptr_note_context_switch() does before promoting > > > to backup slot, it checks for a NULL slot->addr, which is exactly > > > what is missing from hazptr_detach. > > > > > > With this in place there would be no need to explicitly check the > > > hazptr_acquire() return value before calling hazptr_detach(). > > > > > > hazptr_release() has a early return check for NULL addr as well, so it > > > makes sense that detach does an early return (no-op) similarly. > > > > > > > You shall kill me for this!! > > > > Could you perhaps do a splat in ze commit description pls? > > The splat is available at the "Closes" URL below. I'm not sure whether > we should duplicate this verbose information ? > > Paul ? I am fine either way, as long as the information is reasonably easily accessible. Which is the case either way. ;-) Thanx, Paul > Thanks, > > Mathieu > > > > > > > > Fixes: 6357ec235c59 ("hazptrtorture: Fix hazptr ownership issue") > > > Reported-by: kernel test robot <yi1.lai@intel.com> > > > Closes: https://lore.kernel.org/oe-lkp/202608130915.62b53936-lkp@intel.com > > > Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> > > > Reviewed-by: Bradley Morgan <brads@mainlining.org> > > > Cc: Paul E. McKenney <paulmck@kernel.org> > > > Cc: Boqun Feng <boqun@kernel.org> > > > Cc: Bradley Morgan <brads@mainlining.org> > > > Cc: <rcu@vger.kernel.org> > > > Cc: <lkmm@lists.linux.dev> > > > --- > > > include/linux/hazptr.h | 4 +++- > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > diff --git a/include/linux/hazptr.h b/include/linux/hazptr.h > > > index 43122c5673bd..d1670121947a 100644 > > > --- a/include/linux/hazptr.h > > > +++ b/include/linux/hazptr.h > > > @@ -160,10 +160,12 @@ void hazptr_detach(struct hazptr_ctx *ctx) > > > struct hazptr_slot *slot; > > > > > > guard(preempt)(); > > > + slot = ctx->slot; > > > + if (!slot->addr) > > > + return; > > > #ifdef CONFIG_HAZPTR_DEBUG > > > ctx->detach_task = ctx->detach_cpu = true; > > > #endif > > > - slot = ctx->slot; > > > if (unlikely(hazptr_slot_is_backup(ctx, slot))) > > > return; > > > hazptr_promote_to_backup_slot(ctx, slot); > > > > > > > --- Thanks! > > https://lore.kernel.org/all/EE579805-42F2-4C58-B752-F28779EEB717@grrlz.net/ > > > -- > Mathieu Desnoyers > EfficiOS Inc. > https://www.efficios.com ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] hazptr: handle NULL address in hazptr_detach 2026-09-08 16:22 ` Paul E. McKenney @ 2026-09-08 16:31 ` Bradley Morgan 2026-09-08 16:53 ` Mathieu Desnoyers 1 sibling, 0 replies; 10+ messages in thread From: Bradley Morgan @ 2026-09-08 16:31 UTC (permalink / raw) To: paulmck, Paul E. McKenney, Mathieu Desnoyers Cc: linux-kernel, kernel test robot, Boqun Feng, rcu, lkmm On 8 September 2026 17:22:09 BST, "Paul E. McKenney" <paulmck@kernel.org> wrote: >On Tue, Sep 08, 2026 at 11:29:31AM -0400, Mathieu Desnoyers wrote: >> On 2026-09-08 11:24, Bradley Morgan wrote: >> > On 8 September 2026 16:22:14 BST, Mathieu Desnoyers >> > <mathieu.desnoyers@efficios.com> wrote: >> > > When hazptr_acquire loads a NULL pointer, it sets: >> > > >> > > - slot_item->slot.addr = NULL, >> > > - slot_item->ctx.ctx = ctx >> > > - ctx->slot = slot >> > > >> > > And it returns NULL. >> > > >> > > Then hazptr_detach is called on this ctx, it will act on the ctx as >if >> > > needed to be promoted to backup slot, even though it has a NULL >addr. >> > > >> > > Looking at what hazptr_note_context_switch() does before promoting >> > > to backup slot, it checks for a NULL slot->addr, which is exactly >> > > what is missing from hazptr_detach. >> > > >> > > With this in place there would be no need to explicitly check the >> > > hazptr_acquire() return value before calling hazptr_detach(). >> > > >> > > hazptr_release() has a early return check for NULL addr as well, so >it >> > > makes sense that detach does an early return (no-op) similarly. >> > > >> > >> > You shall kill me for this!! >> > >> > Could you perhaps do a splat in ze commit description pls? >> >> The splat is available at the "Closes" URL below. I'm not sure whether >> we should duplicate this verbose information ? >> >> Paul ? > >I am fine either way, as long as the information is reasonably easily >accessible. Which is the case either way. ;-) > > Thanx, Paul > Hi, what do you reckon, do you wanna merge it? I can co develop hazptr if u want, it's school season for me but oh well. Commit to Linux anyway! >> Thanks, >> >> Mathieu >> >> > >> > >> > > Fixes: 6357ec235c59 ("hazptrtorture: Fix hazptr ownership issue") >> > > Reported-by: kernel test robot <yi1.lai@intel.com> >> > > Closes: >https://lore.kernel.org/oe-lkp/202608130915.62b53936-lkp@intel.com >> > > Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> >> > > Reviewed-by: Bradley Morgan <brads@mainlining.org> >> > > Cc: Paul E. McKenney <paulmck@kernel.org> >> > > Cc: Boqun Feng <boqun@kernel.org> >> > > Cc: Bradley Morgan <brads@mainlining.org> >> > > Cc: <rcu@vger.kernel.org> >> > > Cc: <lkmm@lists.linux.dev> >> > > --- >> > > include/linux/hazptr.h | 4 +++- >> > > 1 file changed, 3 insertions(+), 1 deletion(-) >> > > >> > > diff --git a/include/linux/hazptr.h b/include/linux/hazptr.h >> > > index 43122c5673bd..d1670121947a 100644 >> > > --- a/include/linux/hazptr.h >> > > +++ b/include/linux/hazptr.h >> > > @@ -160,10 +160,12 @@ void hazptr_detach(struct hazptr_ctx *ctx) >> > > struct hazptr_slot *slot; >> > > >> > > guard(preempt)(); >> > > + slot = ctx->slot; >> > > + if (!slot->addr) >> > > + return; >> > > #ifdef CONFIG_HAZPTR_DEBUG >> > > ctx->detach_task = ctx->detach_cpu = true; >> > > #endif >> > > - slot = ctx->slot; >> > > if (unlikely(hazptr_slot_is_backup(ctx, slot))) >> > > return; >> > > hazptr_promote_to_backup_slot(ctx, slot); >> > > >> > >> > --- Thanks! >> > >https://lore.kernel.org/all/EE579805-42F2-4C58-B752-F28779EEB717@grrlz.net/ >> >> >> -- >> Mathieu Desnoyers >> EfficiOS Inc. >> https://www.efficios.com --- Thanks! https://lore.kernel.org/all/EE579805-42F2-4C58-B752-F28779EEB717@grrlz.net/ ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] hazptr: handle NULL address in hazptr_detach 2026-09-08 16:22 ` Paul E. McKenney 2026-09-08 16:31 ` Bradley Morgan @ 2026-09-08 16:53 ` Mathieu Desnoyers 2026-09-10 3:14 ` Paul E. McKenney 1 sibling, 1 reply; 10+ messages in thread From: Mathieu Desnoyers @ 2026-09-08 16:53 UTC (permalink / raw) To: paulmck Cc: Bradley Morgan, linux-kernel, kernel test robot, Boqun Feng, rcu, lkmm On 2026-09-08 12:22, Paul E. McKenney wrote: > On Tue, Sep 08, 2026 at 11:29:31AM -0400, Mathieu Desnoyers wrote: >> On 2026-09-08 11:24, Bradley Morgan wrote: >>> On 8 September 2026 16:22:14 BST, Mathieu Desnoyers >>> <mathieu.desnoyers@efficios.com> wrote: >>>> When hazptr_acquire loads a NULL pointer, it sets: >>>> >>>> - slot_item->slot.addr = NULL, >>>> - slot_item->ctx.ctx = ctx >>>> - ctx->slot = slot >>>> >>>> And it returns NULL. >>>> >>>> Then hazptr_detach is called on this ctx, it will act on the ctx as if >>>> needed to be promoted to backup slot, even though it has a NULL addr. >>>> >>>> Looking at what hazptr_note_context_switch() does before promoting >>>> to backup slot, it checks for a NULL slot->addr, which is exactly >>>> what is missing from hazptr_detach. >>>> >>>> With this in place there would be no need to explicitly check the >>>> hazptr_acquire() return value before calling hazptr_detach(). >>>> >>>> hazptr_release() has a early return check for NULL addr as well, so it >>>> makes sense that detach does an early return (no-op) similarly. >>>> >>> >>> You shall kill me for this!! >>> >>> Could you perhaps do a splat in ze commit description pls? >> >> The splat is available at the "Closes" URL below. I'm not sure whether >> we should duplicate this verbose information ? >> >> Paul ? > > I am fine either way, as long as the information is reasonably easily > accessible. Which is the case either way. ;-) Then I favor a concise commit message, leaving the splat details to the "Closes" link. Feel free to pick up this patch for testing. Thank you! Mathieu -- Mathieu Desnoyers EfficiOS Inc. https://www.efficios.com ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] hazptr: handle NULL address in hazptr_detach 2026-09-08 16:53 ` Mathieu Desnoyers @ 2026-09-10 3:14 ` Paul E. McKenney 0 siblings, 0 replies; 10+ messages in thread From: Paul E. McKenney @ 2026-09-10 3:14 UTC (permalink / raw) To: Mathieu Desnoyers Cc: Bradley Morgan, linux-kernel, kernel test robot, Boqun Feng, rcu, lkmm On Tue, Sep 08, 2026 at 12:53:52PM -0400, Mathieu Desnoyers wrote: > On 2026-09-08 12:22, Paul E. McKenney wrote: > > On Tue, Sep 08, 2026 at 11:29:31AM -0400, Mathieu Desnoyers wrote: > > > On 2026-09-08 11:24, Bradley Morgan wrote: > > > > On 8 September 2026 16:22:14 BST, Mathieu Desnoyers > > > > <mathieu.desnoyers@efficios.com> wrote: > > > > > When hazptr_acquire loads a NULL pointer, it sets: > > > > > > > > > > - slot_item->slot.addr = NULL, > > > > > - slot_item->ctx.ctx = ctx > > > > > - ctx->slot = slot > > > > > > > > > > And it returns NULL. > > > > > > > > > > Then hazptr_detach is called on this ctx, it will act on the ctx as if > > > > > needed to be promoted to backup slot, even though it has a NULL addr. > > > > > > > > > > Looking at what hazptr_note_context_switch() does before promoting > > > > > to backup slot, it checks for a NULL slot->addr, which is exactly > > > > > what is missing from hazptr_detach. > > > > > > > > > > With this in place there would be no need to explicitly check the > > > > > hazptr_acquire() return value before calling hazptr_detach(). > > > > > > > > > > hazptr_release() has a early return check for NULL addr as well, so it > > > > > makes sense that detach does an early return (no-op) similarly. > > > > > > > > > > > > > You shall kill me for this!! > > > > > > > > Could you perhaps do a splat in ze commit description pls? > > > > > > The splat is available at the "Closes" URL below. I'm not sure whether > > > we should duplicate this verbose information ? > > > > > > Paul ? > > > > I am fine either way, as long as the information is reasonably easily > > accessible. Which is the case either way. ;-) > > Then I favor a concise commit message, leaving the splat details to the > "Closes" link. Feel free to pick up this patch for testing. > > Thank you! Done, thank you! With this applied, the CPU-hotplug-related failures are gone. Or at least much lower probability. ;-) Thanx, Paul ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] hazptr: handle NULL address in hazptr_detach 2026-09-08 15:22 [PATCH] hazptr: handle NULL address in hazptr_detach Mathieu Desnoyers 2026-09-08 15:24 ` Bradley Morgan @ 2026-09-10 2:16 ` kernel test robot 2026-09-10 3:11 ` Paul E. McKenney 1 sibling, 1 reply; 10+ messages in thread From: kernel test robot @ 2026-09-10 2:16 UTC (permalink / raw) To: Mathieu Desnoyers Cc: Paul E . McKenney, linux-kernel, Bradley Morgan, Boqun Feng, rcu, lkmm, yi1.lai On Tue, Sep 08, 2026 at 11:22:14AM -0400, Mathieu Desnoyers wrote: > When hazptr_acquire loads a NULL pointer, it sets: > > - slot_item->slot.addr = NULL, > - slot_item->ctx.ctx = ctx > - ctx->slot = slot > > And it returns NULL. > > Then hazptr_detach is called on this ctx, it will act on the ctx as if > needed to be promoted to backup slot, even though it has a NULL addr. > > Looking at what hazptr_note_context_switch() does before promoting > to backup slot, it checks for a NULL slot->addr, which is exactly > what is missing from hazptr_detach. > > With this in place there would be no need to explicitly check the > hazptr_acquire() return value before calling hazptr_detach(). > > hazptr_release() has a early return check for NULL addr as well, so it > makes sense that detach does an early return (no-op) similarly. > > Fixes: 6357ec235c59 ("hazptrtorture: Fix hazptr ownership issue") > Reported-by: kernel test robot <yi1.lai@intel.com> > Closes: https://lore.kernel.org/oe-lkp/202608130915.62b53936-lkp@intel.com > Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> > Reviewed-by: Bradley Morgan <brads@mainlining.org> > Cc: Paul E. McKenney <paulmck@kernel.org> > Cc: Boqun Feng <boqun@kernel.org> > Cc: Bradley Morgan <brads@mainlining.org> > Cc: <rcu@vger.kernel.org> > Cc: <lkmm@lists.linux.dev> > --- > include/linux/hazptr.h | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/include/linux/hazptr.h b/include/linux/hazptr.h > index 43122c5673bd..d1670121947a 100644 > --- a/include/linux/hazptr.h > +++ b/include/linux/hazptr.h > @@ -160,10 +160,12 @@ void hazptr_detach(struct hazptr_ctx *ctx) > struct hazptr_slot *slot; > > guard(preempt)(); > + slot = ctx->slot; > + if (!slot->addr) > + return; > #ifdef CONFIG_HAZPTR_DEBUG > ctx->detach_task = ctx->detach_cpu = true; > #endif > - slot = ctx->slot; > if (unlikely(hazptr_slot_is_backup(ctx, slot))) > return; > hazptr_promote_to_backup_slot(ctx, slot); > -- > 2.43.0 > Applied this fix patch on top of Pual's v2 RFC patch series. The issue cannot be reproduce. Tested-by: kernel test robot <yi1.lai@intel.com> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] hazptr: handle NULL address in hazptr_detach 2026-09-10 2:16 ` kernel test robot @ 2026-09-10 3:11 ` Paul E. McKenney 0 siblings, 0 replies; 10+ messages in thread From: Paul E. McKenney @ 2026-09-10 3:11 UTC (permalink / raw) To: kernel test robot Cc: Mathieu Desnoyers, linux-kernel, Bradley Morgan, Boqun Feng, rcu, lkmm On Thu, Sep 10, 2026 at 10:16:39AM +0800, kernel test robot wrote: > On Tue, Sep 08, 2026 at 11:22:14AM -0400, Mathieu Desnoyers wrote: > > When hazptr_acquire loads a NULL pointer, it sets: > > > > - slot_item->slot.addr = NULL, > > - slot_item->ctx.ctx = ctx > > - ctx->slot = slot > > > > And it returns NULL. > > > > Then hazptr_detach is called on this ctx, it will act on the ctx as if > > needed to be promoted to backup slot, even though it has a NULL addr. > > > > Looking at what hazptr_note_context_switch() does before promoting > > to backup slot, it checks for a NULL slot->addr, which is exactly > > what is missing from hazptr_detach. > > > > With this in place there would be no need to explicitly check the > > hazptr_acquire() return value before calling hazptr_detach(). > > > > hazptr_release() has a early return check for NULL addr as well, so it > > makes sense that detach does an early return (no-op) similarly. > > > > Fixes: 6357ec235c59 ("hazptrtorture: Fix hazptr ownership issue") > > Reported-by: kernel test robot <yi1.lai@intel.com> > > Closes: https://lore.kernel.org/oe-lkp/202608130915.62b53936-lkp@intel.com > > Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> > > Reviewed-by: Bradley Morgan <brads@mainlining.org> > > Cc: Paul E. McKenney <paulmck@kernel.org> > > Cc: Boqun Feng <boqun@kernel.org> > > Cc: Bradley Morgan <brads@mainlining.org> > > Cc: <rcu@vger.kernel.org> > > Cc: <lkmm@lists.linux.dev> > > --- > > include/linux/hazptr.h | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/include/linux/hazptr.h b/include/linux/hazptr.h > > index 43122c5673bd..d1670121947a 100644 > > --- a/include/linux/hazptr.h > > +++ b/include/linux/hazptr.h > > @@ -160,10 +160,12 @@ void hazptr_detach(struct hazptr_ctx *ctx) > > struct hazptr_slot *slot; > > > > guard(preempt)(); > > + slot = ctx->slot; > > + if (!slot->addr) > > + return; > > #ifdef CONFIG_HAZPTR_DEBUG > > ctx->detach_task = ctx->detach_cpu = true; > > #endif > > - slot = ctx->slot; > > if (unlikely(hazptr_slot_is_backup(ctx, slot))) > > return; > > hazptr_promote_to_backup_slot(ctx, slot); > > -- > > 2.43.0 > > > > Applied this fix patch on top of Pual's v2 RFC patch series. The issue > cannot be reproduce. > > Tested-by: kernel test robot <yi1.lai@intel.com> Thank you! I will apply this on my next rebase. Thanx, Paul ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-09-10 3:14 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-09-08 15:22 [PATCH] hazptr: handle NULL address in hazptr_detach Mathieu Desnoyers 2026-09-08 15:24 ` Bradley Morgan 2026-09-08 15:29 ` Mathieu Desnoyers 2026-09-08 15:52 ` Bradley Morgan 2026-09-08 16:22 ` Paul E. McKenney 2026-09-08 16:31 ` Bradley Morgan 2026-09-08 16:53 ` Mathieu Desnoyers 2026-09-10 3:14 ` Paul E. McKenney 2026-09-10 2:16 ` kernel test robot 2026-09-10 3:11 ` Paul E. McKenney
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®