mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [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 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

* 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

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®