[PATCH] resource: fix lost wakeup when waiting for a muxed region

DAI RENJIE via B4 Relay posted 1 patch 1 month, 1 week ago
kernel/resource.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] resource: fix lost wakeup when waiting for a muxed region
Posted by DAI RENJIE via B4 Relay 1 month, 1 week ago
From: DAI RENJIE <drj19981414013@gmail.com>

A task waiting for a muxed region can sleep forever in TASK_UNINTERRUPTIBLE
even though the region it waits for is already free.
__request_region_locked() queues itself on muxed_resource_wait and drops
resource_lock before setting TASK_UNINTERRUPTIBLE, while __release_region()
wakes the queue after dropping the same lock.  A wakeup landing in between
finds TASK_RUNNING, does not match TASK_NORMAL and is discarded; callers
hold a muxed region only across a bounded transaction, so no further
release is coming.  The task is unkillable and its caller never returns.

The window is one store wide, but an interrupt is enough to hold the waiter
in it, and the machine this was seen on runs PREEMPT_DYNAMIC in its
voluntary default.  Since v6.11 spd5118 exports the DDR5 sensors of AMD
boards through i2c-piix4, which takes a muxed region per SMBus transaction;
a third of the in-tree users of request_muxed_region() are hwmon drivers,
so reading a world-readable attribute is all an unprivileged user needs to
drive the contention.  The blocked task sleeps holding the i2c adapter bus
lock, and 27 more piled up behind it.

Fix it by setting the task state before dropping resource_lock, as
prepare_to_wait() does: the releasing side needs resource_lock to unlink
the resource, so it cannot reach the wakeup before the state is published.

Fixes: 8b6d043b7ee2 ("resource: shared I/O region support")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-5
Signed-off-by: DAI RENJIE <drj19981414013@gmail.com>
---
Reproduced by building a kernel with the two orderings selectable at
runtime and a 2ms delay inside the window.  Switching only that knob, a
two-thread barriered reproducer loses the wakeup 200 times out of 200
before the fix and 0 out of 200 after it; without the delay it goes 20000
times through the wait path and loses none.
---
 kernel/resource.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/resource.c b/kernel/resource.c
index 3d17e3196a3e..3604b7152808 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -1350,8 +1350,8 @@ static int __request_region_locked(struct resource *res, struct resource *parent
 		}
 		if (conflict->flags & flags & IORESOURCE_MUXED) {
 			add_wait_queue(&muxed_resource_wait, &wait);
-			write_unlock(&resource_lock);
 			set_current_state(TASK_UNINTERRUPTIBLE);
+			write_unlock(&resource_lock);
 			schedule();
 			remove_wait_queue(&muxed_resource_wait, &wait);
 			write_lock(&resource_lock);

---
base-commit: 818bebeb63dd6bf5f4e07e145f6cdbace520a34c
change-id: 20260821-b4-resource-muxed-lost-wakeup-36d8d804e191

Best regards,
--  
DAI RENJIE <drj19981414013@gmail.com>
Re: [PATCH] resource: fix lost wakeup when waiting for a muxed region
Posted by Andrew Morton 1 month, 1 week ago
On Fri, 21 Aug 2026 14:08:17 +0000 DAI RENJIE via B4 Relay <devnull+drj19981414013.gmail.com@kernel.org> wrote:

> From: DAI RENJIE <drj19981414013@gmail.com>
> 
> A task waiting for a muxed region can sleep forever in TASK_UNINTERRUPTIBLE
> even though the region it waits for is already free.
> __request_region_locked() queues itself on muxed_resource_wait and drops
> resource_lock before setting TASK_UNINTERRUPTIBLE, while __release_region()
> wakes the queue after dropping the same lock.  A wakeup landing in between
> finds TASK_RUNNING, does not match TASK_NORMAL and is discarded; callers
> hold a muxed region only across a bounded transaction, so no further
> release is coming.  The task is unkillable and its caller never returns.
> 
> The window is one store wide, but an interrupt is enough to hold the waiter
> in it, and the machine this was seen on runs PREEMPT_DYNAMIC in its
> voluntary default.  Since v6.11 spd5118 exports the DDR5 sensors of AMD
> boards through i2c-piix4, which takes a muxed region per SMBus transaction;
> a third of the in-tree users of request_muxed_region() are hwmon drivers,
> so reading a world-readable attribute is all an unprivileged user needs to
> drive the contention.  The blocked task sleeps holding the i2c adapter bus
> lock, and 27 more piled up behind it.
> 
> Fix it by setting the task state before dropping resource_lock, as
> prepare_to_wait() does: the releasing side needs resource_lock to unlink
> the resource, so it cannot reach the wakeup before the state is published.
> 
> Fixes: 8b6d043b7ee2 ("resource: shared I/O region support")

16 years ago.

> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: DAI RENJIE <drj19981414013@gmail.com>
> ---
> Reproduced by building a kernel with the two orderings selectable at
> runtime and a 2ms delay inside the window.  Switching only that knob, a
> two-thread barriered reproducer loses the wakeup 200 times out of 200
> before the fix and 0 out of 200 after it; without the delay it goes 20000
> times through the wait path and loses none.
> ---
>  kernel/resource.c | 2 +-

Bjorn, please consider a MAINTAINERS entry?

> --- a/kernel/resource.c
> +++ b/kernel/resource.c
> @@ -1350,8 +1350,8 @@ static int __request_region_locked(struct resource *res, struct resource *parent
>  		}
>  		if (conflict->flags & flags & IORESOURCE_MUXED) {
>  			add_wait_queue(&muxed_resource_wait, &wait);
> -			write_unlock(&resource_lock);
>  			set_current_state(TASK_UNINTERRUPTIBLE);
> +			write_unlock(&resource_lock);
>  			schedule();
>  			remove_wait_queue(&muxed_resource_wait, &wait);
>  			write_lock(&resource_lock);

Yup, that's a basic waitqueue usage bug.
Re: [PATCH] resource: fix lost wakeup when waiting for a muxed region
Posted by Bjorn Helgaas 1 month ago
[+to Ilpo]
On Fri, Aug 21, 2026 at 11:34:57AM -0700, Andrew Morton wrote:
> On Fri, 21 Aug 2026 14:08:17 +0000 DAI RENJIE via B4 Relay <devnull+drj19981414013.gmail.com@kernel.org> wrote:
> 
> > From: DAI RENJIE <drj19981414013@gmail.com>
> > 
> > A task waiting for a muxed region can sleep forever in TASK_UNINTERRUPTIBLE
> > even though the region it waits for is already free.
> > __request_region_locked() queues itself on muxed_resource_wait and drops
> > resource_lock before setting TASK_UNINTERRUPTIBLE, while __release_region()
> > wakes the queue after dropping the same lock.  A wakeup landing in between
> > finds TASK_RUNNING, does not match TASK_NORMAL and is discarded; callers
> > hold a muxed region only across a bounded transaction, so no further
> > release is coming.  The task is unkillable and its caller never returns.
> > 
> > The window is one store wide, but an interrupt is enough to hold the waiter
> > in it, and the machine this was seen on runs PREEMPT_DYNAMIC in its
> > voluntary default.  Since v6.11 spd5118 exports the DDR5 sensors of AMD
> > boards through i2c-piix4, which takes a muxed region per SMBus transaction;
> > a third of the in-tree users of request_muxed_region() are hwmon drivers,
> > so reading a world-readable attribute is all an unprivileged user needs to
> > drive the contention.  The blocked task sleeps holding the i2c adapter bus
> > lock, and 27 more piled up behind it.
> > 
> > Fix it by setting the task state before dropping resource_lock, as
> > prepare_to_wait() does: the releasing side needs resource_lock to unlink
> > the resource, so it cannot reach the wakeup before the state is published.
> > 
> > Fixes: 8b6d043b7ee2 ("resource: shared I/O region support")
> 
> 16 years ago.
> 
> > Cc: stable@vger.kernel.org
> > Assisted-by: Claude:claude-opus-5
> > Signed-off-by: DAI RENJIE <drj19981414013@gmail.com>
> > ---
> > Reproduced by building a kernel with the two orderings selectable at
> > runtime and a 2ms delay inside the window.  Switching only that knob, a
> > two-thread barriered reproducer loses the wakeup 200 times out of 200
> > before the fix and 0 out of 200 after it; without the delay it goes 20000
> > times through the wait path and loses none.
> > ---
> >  kernel/resource.c | 2 +-
> 
> Bjorn, please consider a MAINTAINERS entry?

Maybe you'd be interested, Ilpo, since you're doing so much PCI
resource work?

> > --- a/kernel/resource.c
> > +++ b/kernel/resource.c
> > @@ -1350,8 +1350,8 @@ static int __request_region_locked(struct resource *res, struct resource *parent
> >  		}
> >  		if (conflict->flags & flags & IORESOURCE_MUXED) {
> >  			add_wait_queue(&muxed_resource_wait, &wait);
> > -			write_unlock(&resource_lock);
> >  			set_current_state(TASK_UNINTERRUPTIBLE);
> > +			write_unlock(&resource_lock);
> >  			schedule();
> >  			remove_wait_queue(&muxed_resource_wait, &wait);
> >  			write_lock(&resource_lock);
> 
> Yup, that's a basic waitqueue usage bug.  
>
Re: [PATCH] resource: fix lost wakeup when waiting for a muxed region
Posted by Bradley Morgan 1 month, 1 week ago
On 21 August 2026 19:34:57 BST, Andrew Morton <akpm@linux-foundation.org>
wrote:
>On Fri, 21 Aug 2026 14:08:17 +0000 DAI RENJIE via B4 Relay
><devnull+drj19981414013.gmail.com@kernel.org> wrote:
>
>> From: DAI RENJIE <drj19981414013@gmail.com>
>> 
>> A task waiting for a muxed region can sleep forever in
>TASK_UNINTERRUPTIBLE
>> even though the region it waits for is already free.
>> __request_region_locked() queues itself on muxed_resource_wait and drops
>> resource_lock before setting TASK_UNINTERRUPTIBLE, while
>__release_region()
>> wakes the queue after dropping the same lock.  A wakeup landing in
>between
>> finds TASK_RUNNING, does not match TASK_NORMAL and is discarded; callers
>> hold a muxed region only across a bounded transaction, so no further
>> release is coming.  The task is unkillable and its caller never returns.
>> 
>> The window is one store wide, but an interrupt is enough to hold the
>waiter
>> in it, and the machine this was seen on runs PREEMPT_DYNAMIC in its
>> voluntary default.  Since v6.11 spd5118 exports the DDR5 sensors of AMD
>> boards through i2c-piix4, which takes a muxed region per SMBus
>transaction;
>> a third of the in-tree users of request_muxed_region() are hwmon
>drivers,
>> so reading a world-readable attribute is all an unprivileged user needs
>to
>> drive the contention.  The blocked task sleeps holding the i2c adapter
>bus
>> lock, and 27 more piled up behind it.
>> 
>> Fix it by setting the task state before dropping resource_lock, as
>> prepare_to_wait() does: the releasing side needs resource_lock to unlink
>> the resource, so it cannot reach the wakeup before the state is
>published.
>> 
>> Fixes: 8b6d043b7ee2 ("resource: shared I/O region support")
>
>16 years ago.
>
>> Cc: stable@vger.kernel.org
>> Assisted-by: Claude:claude-opus-5
>> Signed-off-by: DAI RENJIE <drj19981414013@gmail.com>
>> ---
>> Reproduced by building a kernel with the two orderings selectable at
>> runtime and a 2ms delay inside the window.  Switching only that knob, a
>> two-thread barriered reproducer loses the wakeup 200 times out of 200
>> before the fix and 0 out of 200 after it; without the delay it goes
>20000
>> times through the wait path and loses none.
>> ---
>>  kernel/resource.c | 2 +-
>
>Bjorn, please consider a MAINTAINERS entry?
>

I wouldn't mind stepping up, what do you reckon?

Me and Bjorn?

>> --- a/kernel/resource.c
>> +++ b/kernel/resource.c
>> @@ -1350,8 +1350,8 @@ static int __request_region_locked(struct resource
>*res, struct resource *parent
>>  		}
>>  		if (conflict->flags & flags & IORESOURCE_MUXED) {
>>  			add_wait_queue(&muxed_resource_wait, &wait);
>> -			write_unlock(&resource_lock);
>>  			set_current_state(TASK_UNINTERRUPTIBLE);
>> +			write_unlock(&resource_lock);
>>  			schedule();
>>  			remove_wait_queue(&muxed_resource_wait, &wait);
>>  			write_lock(&resource_lock);
>
>Yup, that's a basic waitqueue usage bug.  
>
>

Thanks!
Re: [PATCH] resource: fix lost wakeup when waiting for a muxed region
Posted by Bradley Morgan 1 month, 1 week ago
On 21 August 2026 15:08:17 BST, DAI RENJIE via B4 Relay
<devnull+drj19981414013.gmail.com@kernel.org> wrote:
>From: DAI RENJIE <drj19981414013@gmail.com>
>

+CC akpm

>A task waiting for a muxed region can sleep forever in
>TASK_UNINTERRUPTIBLE
>even though the region it waits for is already free.
>__request_region_locked() queues itself on muxed_resource_wait and drops
>resource_lock before setting TASK_UNINTERRUPTIBLE, while
>__release_region()
>wakes the queue after dropping the same lock.  A wakeup landing in between
>finds TASK_RUNNING, does not match TASK_NORMAL and is discarded; callers
>hold a muxed region only across a bounded transaction, so no further
>release is coming.  The task is unkillable and its caller never returns.
>

Wow! Real life use case?

>The window is one store wide, but an interrupt is enough to hold the
>waiter
>in it, and the machine this was seen on runs PREEMPT_DYNAMIC in its
>voluntary default.  Since v6.11 spd5118 exports the DDR5 sensors of AMD
>boards through i2c-piix4, which takes a muxed region per SMBus
>transaction;
>a third of the in-tree users of request_muxed_region() are hwmon drivers,
>so reading a world-readable attribute is all an unprivileged user needs to
>drive the contention.  The blocked task sleeps holding the i2c adapter bus
>lock, and 27 more piled up behind it.
>

okay, fair enough

>Fix it by setting the task state before dropping resource_lock, as
>prepare_to_wait() does: the releasing side needs resource_lock to unlink
>the resource, so it cannot reach the wakeup before the state is published.
>

Good!

>Fixes: 8b6d043b7ee2 ("resource: shared I/O region support")
>Cc: stable@vger.kernel.org
>Assisted-by: Claude:claude-opus-5

Love you declared AI!

I reviewed this vigourisly, nothing should be wrong!

Reviewed-by: Bradley Morgan <include@grrlz.net>


>Signed-off-by: DAI RENJIE <drj19981414013@gmail.com>
>---
>Reproduced by building a kernel with the two orderings selectable at
>runtime and a 2ms delay inside the window.  Switching only that knob, a
>two-thread barriered reproducer loses the wakeup 200 times out of 200
>before the fix and 0 out of 200 after it; without the delay it goes 20000
>times through the wait path and loses none.

Stable folks will love this

>---
> kernel/resource.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/kernel/resource.c b/kernel/resource.c
>index 3d17e3196a3e..3604b7152808 100644
>--- a/kernel/resource.c
>+++ b/kernel/resource.c
>@@ -1350,8 +1350,8 @@ static int __request_region_locked(struct resource *res, struct resource *parent
> 		}
> 		if (conflict->flags & flags & IORESOURCE_MUXED) {
> 			add_wait_queue(&muxed_resource_wait, &wait);
>-			write_unlock(&resource_lock);
> 			set_current_state(TASK_UNINTERRUPTIBLE);
>+			write_unlock(&resource_lock);

wow, good!

> 			schedule();
> 			remove_wait_queue(&muxed_resource_wait, &wait);
> 			write_lock(&resource_lock);
>
>---
>base-commit: 818bebeb63dd6bf5f4e07e145f6cdbace520a34c
>change-id: 20260821-b4-resource-muxed-lost-wakeup-36d8d804e191
>
>Best regards,
>--  
>DAI RENJIE <drj19981414013@gmail.com>
>
>
>

Thanks!