[PATCH] locking/rwsem: Fix logic error in rwsem_del_waiter()

[PATCH] locking/rwsem: Fix logic error in rwsem_del_waiter()

[Andrei Vagin]

Commit 1ea4b473504b ("locking/rwsem: Remove the list_head from struct
rw_semaphore") introduced a logic error in rwsem_del_waiter().

The root cause of this issue is an inconsistency in the return values of
__rwsem_del_waiter() and rwsem_del_waiter(). Specifically,
__rwsem_del_waiter() returns true when the wait list becomes empty,
whereas rwsem_del_waiter() is supposed to return true if the wait list
is NOT empty.

This caused a null pointer dereference in rwsem_mark_wake() because it
was being called when sem->first_waiter was NULL.

Cc: Matthew Wilcox (Oracle) <willy@infradead.org>
Reported-by: syzbot+3d2ff92c67127d337463@syzkaller.appspotmail.com
Tested-by: syzbot+3d2ff92c67127d337463@syzkaller.appspotmail.com
Fixes: 1ea4b473504b ("locking/rwsem: Remove the list_head from struct rw_semaphore")
Signed-off-by: Andrei Vagin <avagin@google.com>
---
 kernel/locking/rwsem.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/locking/rwsem.c b/kernel/locking/rwsem.c
index ba4cb74de064..bf647097369c 100644
--- a/kernel/locking/rwsem.c
+++ b/kernel/locking/rwsem.c
@@ -370,7 +370,7 @@ bool __rwsem_del_waiter(struct rw_semaphore *sem, struct rwsem_waiter *waiter)
 {
 	if (list_empty(&waiter->list)) {
 		sem->first_waiter = NULL;
-		return true;
+		return false;
 	}
 
 	if (sem->first_waiter == waiter) {
@@ -379,7 +379,7 @@ bool __rwsem_del_waiter(struct rw_semaphore *sem, struct rwsem_waiter *waiter)
 	}
 	list_del(&waiter->list);
 
-	return false;
+	return true;
 }
 
 /*
-- 
2.53.0.851.ga537e3e6e9-goog

Re: [PATCH] locking/rwsem: Fix logic error in rwsem_del_waiter()

[Peter Zijlstra]

On Sat, Mar 14, 2026 at 06:26:07PM +0000, Andrei Vagin wrote:
> Commit 1ea4b473504b ("locking/rwsem: Remove the list_head from struct
> rw_semaphore") introduced a logic error in rwsem_del_waiter().
> 
> The root cause of this issue is an inconsistency in the return values of
> __rwsem_del_waiter() and rwsem_del_waiter(). Specifically,
> __rwsem_del_waiter() returns true when the wait list becomes empty,
> whereas rwsem_del_waiter() is supposed to return true if the wait list
> is NOT empty.
> 
> This caused a null pointer dereference in rwsem_mark_wake() because it
> was being called when sem->first_waiter was NULL.
> 
> Cc: Matthew Wilcox (Oracle) <willy@infradead.org>
> Reported-by: syzbot+3d2ff92c67127d337463@syzkaller.appspotmail.com
> Tested-by: syzbot+3d2ff92c67127d337463@syzkaller.appspotmail.com
> Fixes: 1ea4b473504b ("locking/rwsem: Remove the list_head from struct rw_semaphore")
> Signed-off-by: Andrei Vagin <avagin@google.com>

Bah, indeed. Thanks!

Re: [PATCH] locking/rwsem: Fix logic error in rwsem_del_waiter()

[Waiman Long]

On 3/16/26 1:34 PM, Waiman Long wrote:
> On 3/14/26 2:26 PM, Andrei Vagin wrote:
>> Commit 1ea4b473504b ("locking/rwsem: Remove the list_head from struct
>> rw_semaphore") introduced a logic error in rwsem_del_waiter().
>>
>> The root cause of this issue is an inconsistency in the return values of
>> __rwsem_del_waiter() and rwsem_del_waiter(). Specifically,
>> __rwsem_del_waiter() returns true when the wait list becomes empty,
>> whereas rwsem_del_waiter() is supposed to return true if the wait list
>> is NOT empty.
>>
>> This caused a null pointer dereference in rwsem_mark_wake() because it
>> was being called when sem->first_waiter was NULL.
>>
>> Cc: Matthew Wilcox (Oracle)<willy@infradead.org>
>> Reported-by:syzbot+3d2ff92c67127d337463@syzkaller.appspotmail.com
>> Tested-by:syzbot+3d2ff92c67127d337463@syzkaller.appspotmail.com
>> Fixes: 1ea4b473504b ("locking/rwsem: Remove the list_head from struct rw_semaphore")
>> Signed-off-by: Andrei Vagin<avagin@google.com>
>> ---
>>   kernel/locking/rwsem.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/kernel/locking/rwsem.c b/kernel/locking/rwsem.c
>> index ba4cb74de064..bf647097369c 100644
>> --- a/kernel/locking/rwsem.c
>> +++ b/kernel/locking/rwsem.c
>> @@ -370,7 +370,7 @@ bool __rwsem_del_waiter(struct rw_semaphore *sem, struct rwsem_waiter *waiter)
>>   {
>>   	if (list_empty(&waiter->list)) {
>>   		sem->first_waiter = NULL;
>> -		return true;
>> +		return false;
>>   	}
>>   
>>   	if (sem->first_waiter == waiter) {
>> @@ -379,7 +379,7 @@ bool __rwsem_del_waiter(struct rw_semaphore *sem, struct rwsem_waiter *waiter)
>>   	}
>>   	list_del(&waiter->list);
>>   
>> -	return false;
>> +	return true;
>>   }
>>   
>>   /*
>
> It will be better if we also document what does the return value of 
> __rwsem_del_waiter() means as the we can't guess from the function 
> name itself. Other that that,
>
> Reviewed-by: Waiman Long <longman@redhat.com>
>
Thinking a bit more about it. I think it will be better to not return a 
value in __rwsem_del_waiter() at all. Something like

diff --git a/kernel/locking/rwsem.c b/kernel/locking/rwsem.c index 
ba4cb74de064..ce57ad3c1120 100644 --- a/kernel/locking/rwsem.c +++ 
b/kernel/locking/rwsem.c @@ -365,12 +365,11 @@ enum rwsem_wake_type { 
#define MAX_READERS_WAKEUP 0x100 static inline -bool 
__rwsem_del_waiter(struct rw_semaphore *sem, struct rwsem_waiter 
*waiter) +void __rwsem_del_waiter(struct rw_semaphore *sem, struct 
rwsem_waiter *waiter) __must_hold(&sem->wait_lock) { if 
(list_empty(&waiter->list)) { sem->first_waiter = NULL; - return true; } 
if (sem->first_waiter == waiter) { @@ -378,8 +377,6 @@ bool 
__rwsem_del_waiter(struct rw_semaphore *sem, struct rwsem_waiter 
*waiter) struct rwsem_waiter, list); } list_del(&waiter->list); - - 
return false; } /* @@ -394,7 +391,8 @@ static inline bool 
rwsem_del_waiter(struct rw_semaphore *sem, struct rwsem_waiter *waiter) 
{ lockdep_assert_held(&sem->wait_lock); - if (__rwsem_del_waiter(sem, 
waiter)) + __rwsem_del_waiter(sem, waiter); + if 
(rwsem_is_contended(sem)) return true; 
atomic_long_andnot(RWSEM_FLAG_HANDOFF | RWSEM_FLAG_WAITERS, 
&sem->count); return false;

Cheers, Longman

Re: [PATCH] locking/rwsem: Fix logic error in rwsem_del_waiter()

[Waiman Long]

On 3/16/26 1:49 PM, Waiman Long wrote:
> On 3/16/26 1:34 PM, Waiman Long wrote:
>> On 3/14/26 2:26 PM, Andrei Vagin wrote:
>>> Commit 1ea4b473504b ("locking/rwsem: Remove the list_head from struct
>>> rw_semaphore") introduced a logic error in rwsem_del_waiter().
>>>
>>> The root cause of this issue is an inconsistency in the return 
>>> values of
>>> __rwsem_del_waiter() and rwsem_del_waiter(). Specifically,
>>> __rwsem_del_waiter() returns true when the wait list becomes empty,
>>> whereas rwsem_del_waiter() is supposed to return true if the wait list
>>> is NOT empty.
>>>
>>> This caused a null pointer dereference in rwsem_mark_wake() because it
>>> was being called when sem->first_waiter was NULL.
>>>
>>> Cc: Matthew Wilcox (Oracle)<willy@infradead.org>
>>> Reported-by:syzbot+3d2ff92c67127d337463@syzkaller.appspotmail.com
>>> Tested-by:syzbot+3d2ff92c67127d337463@syzkaller.appspotmail.com
>>> Fixes: 1ea4b473504b ("locking/rwsem: Remove the list_head from 
>>> struct rw_semaphore")
>>> Signed-off-by: Andrei Vagin<avagin@google.com>
>>> ---
>>>   kernel/locking/rwsem.c | 4 ++--
>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/kernel/locking/rwsem.c b/kernel/locking/rwsem.c
>>> index ba4cb74de064..bf647097369c 100644
>>> --- a/kernel/locking/rwsem.c
>>> +++ b/kernel/locking/rwsem.c
>>> @@ -370,7 +370,7 @@ bool __rwsem_del_waiter(struct rw_semaphore 
>>> *sem, struct rwsem_waiter *waiter)
>>>   {
>>>       if (list_empty(&waiter->list)) {
>>>           sem->first_waiter = NULL;
>>> -        return true;
>>> +        return false;
>>>       }
>>>         if (sem->first_waiter == waiter) {
>>> @@ -379,7 +379,7 @@ bool __rwsem_del_waiter(struct rw_semaphore 
>>> *sem, struct rwsem_waiter *waiter)
>>>       }
>>>       list_del(&waiter->list);
>>>   -    return false;
>>> +    return true;
>>>   }
>>>     /*
>>
>> It will be better if we also document what does the return value of 
>> __rwsem_del_waiter() means as the we can't guess from the function 
>> name itself. Other that that,
>>
>> Reviewed-by: Waiman Long <longman@redhat.com>
>>
> Thinking a bit more about it. I think it will be better to not return 
> a value in __rwsem_del_waiter() at all. Something like
>
Sorry, my mailer screwed up the diff. It should be as follows.

Cheers,
Longman

=================================[ Cut here ]============================
diff --git a/kernel/locking/rwsem.c b/kernel/locking/rwsem.c
index ba4cb74de064..ce57ad3c1120 100644
--- a/kernel/locking/rwsem.c
+++ b/kernel/locking/rwsem.c
@@ -365,12 +365,11 @@ enum rwsem_wake_type {
  #define MAX_READERS_WAKEUP     0x100
  
  static inline
-bool __rwsem_del_waiter(struct rw_semaphore *sem, struct rwsem_waiter *waiter)
+void __rwsem_del_waiter(struct rw_semaphore *sem, struct rwsem_waiter *waiter)
         __must_hold(&sem->wait_lock)
  {
         if (list_empty(&waiter->list)) {
                 sem->first_waiter = NULL;
-               return true;
         }
  
         if (sem->first_waiter == waiter) {
@@ -378,8 +377,6 @@ bool __rwsem_del_waiter(struct rw_semaphore *sem, struct rw>
                                                      struct rwsem_waiter, list);
         }
         list_del(&waiter->list);
-
-       return false;
  }
  
  /*
@@ -394,7 +391,8 @@ static inline bool
  rwsem_del_waiter(struct rw_semaphore *sem, struct rwsem_waiter *waiter)
  {
         lockdep_assert_held(&sem->wait_lock);
-       if (__rwsem_del_waiter(sem, waiter))
+       __rwsem_del_waiter(sem, waiter);
+       if (rwsem_is_contended(sem))
                 return true;
         atomic_long_andnot(RWSEM_FLAG_HANDOFF | RWSEM_FLAG_WAITERS, &sem->count>
         return false;

Re: [PATCH] locking/rwsem: Fix logic error in rwsem_del_waiter()

[Andrei Vagin]

On Mon, Mar 16, 2026 at 12:10 PM Waiman Long <longman@redhat.com> wrote:
>
> On 3/16/26 1:49 PM, Waiman Long wrote:
> > On 3/16/26 1:34 PM, Waiman Long wrote:
> >> On 3/14/26 2:26 PM, Andrei Vagin wrote:
> >>> Commit 1ea4b473504b ("locking/rwsem: Remove the list_head from struct
> >>> rw_semaphore") introduced a logic error in rwsem_del_waiter().
> >>>
> >>> The root cause of this issue is an inconsistency in the return
> >>> values of
> >>> __rwsem_del_waiter() and rwsem_del_waiter(). Specifically,
> >>> __rwsem_del_waiter() returns true when the wait list becomes empty,
> >>> whereas rwsem_del_waiter() is supposed to return true if the wait list
> >>> is NOT empty.
> >>>
> >>> This caused a null pointer dereference in rwsem_mark_wake() because it
> >>> was being called when sem->first_waiter was NULL.
> >>>
> >>> Cc: Matthew Wilcox (Oracle)<willy@infradead.org>
> >>> Reported-by:syzbot+3d2ff92c67127d337463@syzkaller.appspotmail.com
> >>> Tested-by:syzbot+3d2ff92c67127d337463@syzkaller.appspotmail.com
> >>> Fixes: 1ea4b473504b ("locking/rwsem: Remove the list_head from
> >>> struct rw_semaphore")
> >>> Signed-off-by: Andrei Vagin<avagin@google.com>
> >>> ---
> >>>   kernel/locking/rwsem.c | 4 ++--
> >>>   1 file changed, 2 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/kernel/locking/rwsem.c b/kernel/locking/rwsem.c
> >>> index ba4cb74de064..bf647097369c 100644
> >>> --- a/kernel/locking/rwsem.c
> >>> +++ b/kernel/locking/rwsem.c
> >>> @@ -370,7 +370,7 @@ bool __rwsem_del_waiter(struct rw_semaphore
> >>> *sem, struct rwsem_waiter *waiter)
> >>>   {
> >>>       if (list_empty(&waiter->list)) {
> >>>           sem->first_waiter = NULL;
> >>> -        return true;
> >>> +        return false;
> >>>       }
> >>>         if (sem->first_waiter == waiter) {
> >>> @@ -379,7 +379,7 @@ bool __rwsem_del_waiter(struct rw_semaphore
> >>> *sem, struct rwsem_waiter *waiter)
> >>>       }
> >>>       list_del(&waiter->list);
> >>>   -    return false;
> >>> +    return true;
> >>>   }
> >>>     /*
> >>
> >> It will be better if we also document what does the return value of
> >> __rwsem_del_waiter() means as the we can't guess from the function
> >> name itself. Other that that,
> >>
> >> Reviewed-by: Waiman Long <longman@redhat.com>
> >>
> > Thinking a bit more about it. I think it will be better to not return
> > a value in __rwsem_del_waiter() at all. Something like
> >
> Sorry, my mailer screwed up the diff. It should be as follows.
>
> Cheers,
> Longman
>
> =================================[ Cut here ]============================
> diff --git a/kernel/locking/rwsem.c b/kernel/locking/rwsem.c
> index ba4cb74de064..ce57ad3c1120 100644
> --- a/kernel/locking/rwsem.c
> +++ b/kernel/locking/rwsem.c
> @@ -365,12 +365,11 @@ enum rwsem_wake_type {
>   #define MAX_READERS_WAKEUP     0x100
>
>   static inline
> -bool __rwsem_del_waiter(struct rw_semaphore *sem, struct rwsem_waiter *waiter)
> +void __rwsem_del_waiter(struct rw_semaphore *sem, struct rwsem_waiter *waiter)
>          __must_hold(&sem->wait_lock)
>   {
>          if (list_empty(&waiter->list)) {
>                  sem->first_waiter = NULL;
> -               return true;

We still need `return` here.

>          }
>
>          if (sem->first_waiter == waiter) {
> @@ -378,8 +377,6 @@ bool __rwsem_del_waiter(struct rw_semaphore *sem, struct rw>
>                                                       struct rwsem_waiter, list);
>          }
>          list_del(&waiter->list);
> -
> -       return false;
>   }
>
>   /*
> @@ -394,7 +391,8 @@ static inline bool
>   rwsem_del_waiter(struct rw_semaphore *sem, struct rwsem_waiter *waiter)
>   {
>          lockdep_assert_held(&sem->wait_lock);
> -       if (__rwsem_del_waiter(sem, waiter))
> +       __rwsem_del_waiter(sem, waiter);
> +       if (rwsem_is_contended(sem))

I am ok with this approach too. Do you want to send this patch?

Thanks,
Andrei

Re: [PATCH] locking/rwsem: Fix logic error in rwsem_del_waiter()

[Waiman Long]

On 3/17/26 5:23 PM, Andrei Vagin wrote:
> On Mon, Mar 16, 2026 at 12:10 PM Waiman Long <longman@redhat.com> wrote:
>> On 3/16/26 1:49 PM, Waiman Long wrote:
>>> On 3/16/26 1:34 PM, Waiman Long wrote:
>>>> On 3/14/26 2:26 PM, Andrei Vagin wrote:
>>>>> Commit 1ea4b473504b ("locking/rwsem: Remove the list_head from struct
>>>>> rw_semaphore") introduced a logic error in rwsem_del_waiter().
>>>>>
>>>>> The root cause of this issue is an inconsistency in the return
>>>>> values of
>>>>> __rwsem_del_waiter() and rwsem_del_waiter(). Specifically,
>>>>> __rwsem_del_waiter() returns true when the wait list becomes empty,
>>>>> whereas rwsem_del_waiter() is supposed to return true if the wait list
>>>>> is NOT empty.
>>>>>
>>>>> This caused a null pointer dereference in rwsem_mark_wake() because it
>>>>> was being called when sem->first_waiter was NULL.
>>>>>
>>>>> Cc: Matthew Wilcox (Oracle)<willy@infradead.org>
>>>>> Reported-by:syzbot+3d2ff92c67127d337463@syzkaller.appspotmail.com
>>>>> Tested-by:syzbot+3d2ff92c67127d337463@syzkaller.appspotmail.com
>>>>> Fixes: 1ea4b473504b ("locking/rwsem: Remove the list_head from
>>>>> struct rw_semaphore")
>>>>> Signed-off-by: Andrei Vagin<avagin@google.com>
>>>>> ---
>>>>>    kernel/locking/rwsem.c | 4 ++--
>>>>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/kernel/locking/rwsem.c b/kernel/locking/rwsem.c
>>>>> index ba4cb74de064..bf647097369c 100644
>>>>> --- a/kernel/locking/rwsem.c
>>>>> +++ b/kernel/locking/rwsem.c
>>>>> @@ -370,7 +370,7 @@ bool __rwsem_del_waiter(struct rw_semaphore
>>>>> *sem, struct rwsem_waiter *waiter)
>>>>>    {
>>>>>        if (list_empty(&waiter->list)) {
>>>>>            sem->first_waiter = NULL;
>>>>> -        return true;
>>>>> +        return false;
>>>>>        }
>>>>>          if (sem->first_waiter == waiter) {
>>>>> @@ -379,7 +379,7 @@ bool __rwsem_del_waiter(struct rw_semaphore
>>>>> *sem, struct rwsem_waiter *waiter)
>>>>>        }
>>>>>        list_del(&waiter->list);
>>>>>    -    return false;
>>>>> +    return true;
>>>>>    }
>>>>>      /*
>>>> It will be better if we also document what does the return value of
>>>> __rwsem_del_waiter() means as the we can't guess from the function
>>>> name itself. Other that that,
>>>>
>>>> Reviewed-by: Waiman Long <longman@redhat.com>
>>>>
>>> Thinking a bit more about it. I think it will be better to not return
>>> a value in __rwsem_del_waiter() at all. Something like
>>>
>> Sorry, my mailer screwed up the diff. It should be as follows.
>>
>> Cheers,
>> Longman
>>
>> =================================[ Cut here ]============================
>> diff --git a/kernel/locking/rwsem.c b/kernel/locking/rwsem.c
>> index ba4cb74de064..ce57ad3c1120 100644
>> --- a/kernel/locking/rwsem.c
>> +++ b/kernel/locking/rwsem.c
>> @@ -365,12 +365,11 @@ enum rwsem_wake_type {
>>    #define MAX_READERS_WAKEUP     0x100
>>
>>    static inline
>> -bool __rwsem_del_waiter(struct rw_semaphore *sem, struct rwsem_waiter *waiter)
>> +void __rwsem_del_waiter(struct rw_semaphore *sem, struct rwsem_waiter *waiter)
>>           __must_hold(&sem->wait_lock)
>>    {
>>           if (list_empty(&waiter->list)) {
>>                   sem->first_waiter = NULL;
>> -               return true;
> We still need `return` here.
>
>>           }
>>
>>           if (sem->first_waiter == waiter) {
>> @@ -378,8 +377,6 @@ bool __rwsem_del_waiter(struct rw_semaphore *sem, struct rw>
>>                                                        struct rwsem_waiter, list);
>>           }
>>           list_del(&waiter->list);
>> -
>> -       return false;
>>    }
>>
>>    /*
>> @@ -394,7 +391,8 @@ static inline bool
>>    rwsem_del_waiter(struct rw_semaphore *sem, struct rwsem_waiter *waiter)
>>    {
>>           lockdep_assert_held(&sem->wait_lock);
>> -       if (__rwsem_del_waiter(sem, waiter))
>> +       __rwsem_del_waiter(sem, waiter);
>> +       if (rwsem_is_contended(sem))
> I am ok with this approach too. Do you want to send this patch?

Sure. I will send out later tonight or tomorrow morning.

Cheers,
Longman

[tip: locking/core] locking/rwsem: Fix logic error in rwsem_del_waiter()

[tip-bot2 for Andrei Vagin]

The following commit has been merged into the locking/core branch of tip:

Commit-ID:     68bcd8b6e0b10d902f7fc8bf3f08f335f5d1640e
Gitweb:        https://git.kernel.org/tip/68bcd8b6e0b10d902f7fc8bf3f08f335f5d1640e
Author:        Andrei Vagin <avagin@google.com>
AuthorDate:    Sat, 14 Mar 2026 18:26:07 
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Mon, 16 Mar 2026 13:16:48 +01:00

locking/rwsem: Fix logic error in rwsem_del_waiter()

Commit 1ea4b473504b ("locking/rwsem: Remove the list_head from struct
rw_semaphore") introduced a logic error in rwsem_del_waiter().

The root cause of this issue is an inconsistency in the return values of
__rwsem_del_waiter() and rwsem_del_waiter(). Specifically,
__rwsem_del_waiter() returns true when the wait list becomes empty,
whereas rwsem_del_waiter() is supposed to return true if the wait list
is NOT empty.

This caused a null pointer dereference in rwsem_mark_wake() because it
was being called when sem->first_waiter was NULL.

Fixes: 1ea4b473504b ("locking/rwsem: Remove the list_head from struct rw_semaphore")
Reported-by: syzbot+3d2ff92c67127d337463@syzkaller.appspotmail.com
Signed-off-by: Andrei Vagin <avagin@google.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Tested-by: syzbot+3d2ff92c67127d337463@syzkaller.appspotmail.com
Link: https://patch.msgid.link/20260314182607.3343346-1-avagin@google.com
---
 kernel/locking/rwsem.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/locking/rwsem.c b/kernel/locking/rwsem.c
index ba4cb74..bf64709 100644
--- a/kernel/locking/rwsem.c
+++ b/kernel/locking/rwsem.c
@@ -370,7 +370,7 @@ bool __rwsem_del_waiter(struct rw_semaphore *sem, struct rwsem_waiter *waiter)
 {
 	if (list_empty(&waiter->list)) {
 		sem->first_waiter = NULL;
-		return true;
+		return false;
 	}
 
 	if (sem->first_waiter == waiter) {
@@ -379,7 +379,7 @@ bool __rwsem_del_waiter(struct rw_semaphore *sem, struct rwsem_waiter *waiter)
 	}
 	list_del(&waiter->list);
 
-	return false;
+	return true;
 }
 
 /*

Re: [tip: locking/core] locking/rwsem: Fix logic error in rwsem_del_waiter()

[Andrei Vagin]

On Wed, Mar 18, 2026 at 1:02 AM tip-bot2 for Andrei Vagin
<tip-bot2@linutronix.de> wrote:
>
> The following commit has been merged into the locking/core branch of tip:

Peter, Waiman sent another version of this fix:
https://lkml.org/lkml/2026/3/17/2474
I think we need to consider taking that one instead of this one.

Thanks,
Andrei

>
> Commit-ID:     68bcd8b6e0b10d902f7fc8bf3f08f335f5d1640e
> Gitweb:        https://git.kernel.org/tip/68bcd8b6e0b10d902f7fc8bf3f08f335f5d1640e
> Author:        Andrei Vagin <avagin@google.com>
> AuthorDate:    Sat, 14 Mar 2026 18:26:07
> Committer:     Peter Zijlstra <peterz@infradead.org>
> CommitterDate: Mon, 16 Mar 2026 13:16:48 +01:00
>
> locking/rwsem: Fix logic error in rwsem_del_waiter()
>
> Commit 1ea4b473504b ("locking/rwsem: Remove the list_head from struct
> rw_semaphore") introduced a logic error in rwsem_del_waiter().
>
> The root cause of this issue is an inconsistency in the return values of
> __rwsem_del_waiter() and rwsem_del_waiter(). Specifically,
> __rwsem_del_waiter() returns true when the wait list becomes empty,
> whereas rwsem_del_waiter() is supposed to return true if the wait list
> is NOT empty.
>
> This caused a null pointer dereference in rwsem_mark_wake() because it
> was being called when sem->first_waiter was NULL.
>
> Fixes: 1ea4b473504b ("locking/rwsem: Remove the list_head from struct rw_semaphore")
> Reported-by: syzbot+3d2ff92c67127d337463@syzkaller.appspotmail.com
> Signed-off-by: Andrei Vagin <avagin@google.com>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Tested-by: syzbot+3d2ff92c67127d337463@syzkaller.appspotmail.com
> Link: https://patch.msgid.link/20260314182607.3343346-1-avagin@google.com
> ---
>  kernel/locking/rwsem.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/locking/rwsem.c b/kernel/locking/rwsem.c
> index ba4cb74..bf64709 100644
> --- a/kernel/locking/rwsem.c
> +++ b/kernel/locking/rwsem.c
> @@ -370,7 +370,7 @@ bool __rwsem_del_waiter(struct rw_semaphore *sem, struct rwsem_waiter *waiter)
>  {
>         if (list_empty(&waiter->list)) {
>                 sem->first_waiter = NULL;
> -               return true;
> +               return false;
>         }
>
>         if (sem->first_waiter == waiter) {
> @@ -379,7 +379,7 @@ bool __rwsem_del_waiter(struct rw_semaphore *sem, struct rwsem_waiter *waiter)
>         }
>         list_del(&waiter->list);
>
> -       return false;
> +       return true;
>  }
>
>  /*

Re: [tip: locking/core] locking/rwsem: Fix logic error in rwsem_del_waiter()

[Waiman Long]

On 3/18/26 12:49 PM, Andrei Vagin wrote:
> On Wed, Mar 18, 2026 at 1:02 AM tip-bot2 for Andrei Vagin
> <tip-bot2@linutronix.de> wrote:
>> The following commit has been merged into the locking/core branch of tip:
> Peter, Waiman sent another version of this fix:
> https://lkml.org/lkml/2026/3/17/2474
> I think we need to consider taking that one instead of this one.

That is fine. It may be easier for me to send another patch on top of 
the current locking/core branch.

Cheers,
Longman

>
> Thanks,
> Andrei
>
>> Commit-ID:     68bcd8b6e0b10d902f7fc8bf3f08f335f5d1640e
>> Gitweb:        https://git.kernel.org/tip/68bcd8b6e0b10d902f7fc8bf3f08f335f5d1640e
>> Author:        Andrei Vagin <avagin@google.com>
>> AuthorDate:    Sat, 14 Mar 2026 18:26:07
>> Committer:     Peter Zijlstra <peterz@infradead.org>
>> CommitterDate: Mon, 16 Mar 2026 13:16:48 +01:00
>>
>> locking/rwsem: Fix logic error in rwsem_del_waiter()
>>
>> Commit 1ea4b473504b ("locking/rwsem: Remove the list_head from struct
>> rw_semaphore") introduced a logic error in rwsem_del_waiter().
>>
>> The root cause of this issue is an inconsistency in the return values of
>> __rwsem_del_waiter() and rwsem_del_waiter(). Specifically,
>> __rwsem_del_waiter() returns true when the wait list becomes empty,
>> whereas rwsem_del_waiter() is supposed to return true if the wait list
>> is NOT empty.
>>
>> This caused a null pointer dereference in rwsem_mark_wake() because it
>> was being called when sem->first_waiter was NULL.
>>
>> Fixes: 1ea4b473504b ("locking/rwsem: Remove the list_head from struct rw_semaphore")
>> Reported-by: syzbot+3d2ff92c67127d337463@syzkaller.appspotmail.com
>> Signed-off-by: Andrei Vagin <avagin@google.com>
>> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
>> Tested-by: syzbot+3d2ff92c67127d337463@syzkaller.appspotmail.com
>> Link: https://patch.msgid.link/20260314182607.3343346-1-avagin@google.com
>> ---
>>   kernel/locking/rwsem.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/kernel/locking/rwsem.c b/kernel/locking/rwsem.c
>> index ba4cb74..bf64709 100644
>> --- a/kernel/locking/rwsem.c
>> +++ b/kernel/locking/rwsem.c
>> @@ -370,7 +370,7 @@ bool __rwsem_del_waiter(struct rw_semaphore *sem, struct rwsem_waiter *waiter)
>>   {
>>          if (list_empty(&waiter->list)) {
>>                  sem->first_waiter = NULL;
>> -               return true;
>> +               return false;
>>          }
>>
>>          if (sem->first_waiter == waiter) {
>> @@ -379,7 +379,7 @@ bool __rwsem_del_waiter(struct rw_semaphore *sem, struct rwsem_waiter *waiter)
>>          }
>>          list_del(&waiter->list);
>>
>> -       return false;
>> +       return true;
>>   }
>>
>>   /*

Re: [tip: locking/core] locking/rwsem: Fix logic error in rwsem_del_waiter()

[Peter Zijlstra]

On Wed, Mar 18, 2026 at 09:49:17AM -0700, Andrei Vagin wrote:
> On Wed, Mar 18, 2026 at 1:02 AM tip-bot2 for Andrei Vagin
> <tip-bot2@linutronix.de> wrote:
> >
> > The following commit has been merged into the locking/core branch of tip:
> 
> Peter, Waiman sent another version of this fix:
> https://lkml.org/lkml/2026/3/17/2474
> I think we need to consider taking that one instead of this one.

I don't see why that's better. And I already had this in the queue and
through the robots.