All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] kobject: avoid blocking allocation while holding uevent_sock_mutex
@ 2026-08-26  6:46 Tao Yu
  2026-08-26  7:01 ` Greg KH
  0 siblings, 1 reply; 4+ messages in thread
From: Tao Yu @ 2026-08-26  6:46 UTC (permalink / raw)
  To: gregkh
  Cc: rafael, dakr, akpm, driver-core, linux-kernel, Tao Yu,
	syzbot+c22bb42560ec86726aba

uevent_net_broadcast_untagged() still holds uevent_sock_mutex across
netlink_broadcast().  That serializes all untagged uevent senders behind
one global mutex, and it also keeps the mutex held while the netlink
broadcast path may perform blocking memory allocation.

This becomes visible during USB enumeration, where device_add() sends a
KOBJ_ADD uevent from the usb_hub_wq context.  If a listener is slow or
the netlink broadcast path runs into memory pressure, the sender can sit
behind uevent_sock_mutex long enough to trigger hung task reports.

Keep the existing send-side ordering, but move the uevent skb allocation
out of the critical section and use GFP_NOWAIT for the broadcast clones
performed while uevent_sock_mutex is held.  This removes the sleeping
allocation point from the locked region without changing uevent delivery
semantics.

Reported-by: syzbot+c22bb42560ec86726aba@syzkaller.appspotmail.com
Signed-off-by: Tao Yu <tao1.yu@intel.com>
---
 lib/kobject_uevent.c | 28 +++++++++++++++++++---------
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/lib/kobject_uevent.c b/lib/kobject_uevent.c
index ddbc4d7482d24..b8832a5ca583d 100644
--- a/lib/kobject_uevent.c
+++ b/lib/kobject_uevent.c
@@ -311,9 +311,26 @@ static int uevent_net_broadcast_untagged(struct kobj_uevent_env *env,
 {
 	struct sk_buff *skb = NULL;
 	struct uevent_sock *ue_sk;
+	bool has_listeners = false;
 	int retval = 0;
 
-	/* send netlink message */
+	mutex_lock(&uevent_sock_mutex);
+	list_for_each_entry(ue_sk, &uevent_sock_list, list) {
+		if (!netlink_has_listeners(ue_sk->sk, 1))
+			continue;
+
+		has_listeners = true;
+		break;
+	}
+	mutex_unlock(&uevent_sock_mutex);
+
+	if (has_listeners) {
+		skb = alloc_uevent_skb(env, action_string, devpath);
+		if (!skb)
+			return -ENOMEM;
+	}
+
+	/* Keep send-side ordering, but avoid sleeping while holding the mutex. */
 	mutex_lock(&uevent_sock_mutex);
 	list_for_each_entry(ue_sk, &uevent_sock_list, list) {
 		struct sock *uevent_sock = ue_sk->sk;
@@ -321,15 +338,8 @@ static int uevent_net_broadcast_untagged(struct kobj_uevent_env *env,
 		if (!netlink_has_listeners(uevent_sock, 1))
 			continue;
 
-		if (!skb) {
-			retval = -ENOMEM;
-			skb = alloc_uevent_skb(env, action_string, devpath);
-			if (!skb)
-				continue;
-		}
-
 		retval = netlink_broadcast(uevent_sock, skb_get(skb), 0, 1,
-					   GFP_KERNEL);
+					   GFP_NOWAIT);
 		/* ENOBUFS should be handled in userspace */
 		if (retval == -ENOBUFS || retval == -ESRCH)
 			retval = 0;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] kobject: avoid blocking allocation while holding uevent_sock_mutex
  2026-08-26  6:46 [PATCH] kobject: avoid blocking allocation while holding uevent_sock_mutex Tao Yu
@ 2026-08-26  7:01 ` Greg KH
  2026-08-26  7:17   ` Tao Yu
  0 siblings, 1 reply; 4+ messages in thread
From: Greg KH @ 2026-08-26  7:01 UTC (permalink / raw)
  To: Tao Yu
  Cc: rafael, dakr, akpm, driver-core, linux-kernel,
	syzbot+c22bb42560ec86726aba

On Wed, Aug 26, 2026 at 02:46:33PM +0800, Tao Yu wrote:
> uevent_net_broadcast_untagged() still holds uevent_sock_mutex across
> netlink_broadcast().  That serializes all untagged uevent senders behind
> one global mutex, and it also keeps the mutex held while the netlink
> broadcast path may perform blocking memory allocation.
> 
> This becomes visible during USB enumeration, where device_add() sends a
> KOBJ_ADD uevent from the usb_hub_wq context.  If a listener is slow or
> the netlink broadcast path runs into memory pressure, the sender can sit
> behind uevent_sock_mutex long enough to trigger hung task reports.

What has changed to cause this to happen?  USB enumeration is quite
common :)

> Keep the existing send-side ordering, but move the uevent skb allocation
> out of the critical section and use GFP_NOWAIT for the broadcast clones
> performed while uevent_sock_mutex is held.  This removes the sleeping
> allocation point from the locked region without changing uevent delivery
> semantics.
> 
> Reported-by: syzbot+c22bb42560ec86726aba@syzkaller.appspotmail.com

No link?  Was this tested?

No cc: stable?

No Fixes: tag?

> Signed-off-by: Tao Yu <tao1.yu@intel.com>
> ---
>  lib/kobject_uevent.c | 28 +++++++++++++++++++---------
>  1 file changed, 19 insertions(+), 9 deletions(-)
> 
> diff --git a/lib/kobject_uevent.c b/lib/kobject_uevent.c
> index ddbc4d7482d24..b8832a5ca583d 100644
> --- a/lib/kobject_uevent.c
> +++ b/lib/kobject_uevent.c
> @@ -311,9 +311,26 @@ static int uevent_net_broadcast_untagged(struct kobj_uevent_env *env,
>  {
>  	struct sk_buff *skb = NULL;
>  	struct uevent_sock *ue_sk;
> +	bool has_listeners = false;
>  	int retval = 0;
>  
> -	/* send netlink message */
> +	mutex_lock(&uevent_sock_mutex);
> +	list_for_each_entry(ue_sk, &uevent_sock_list, list) {
> +		if (!netlink_has_listeners(ue_sk->sk, 1))
> +			continue;
> +
> +		has_listeners = true;
> +		break;
> +	}
> +	mutex_unlock(&uevent_sock_mutex);

guard()?

> +
> +	if (has_listeners) {

What happens if you get a listner right after the lock is released?

> +		skb = alloc_uevent_skb(env, action_string, devpath);
> +		if (!skb)
> +			return -ENOMEM;
> +	}
> +
> +	/* Keep send-side ordering, but avoid sleeping while holding the mutex. */
>  	mutex_lock(&uevent_sock_mutex);
>  	list_for_each_entry(ue_sk, &uevent_sock_list, list) {
>  		struct sock *uevent_sock = ue_sk->sk;
> @@ -321,15 +338,8 @@ static int uevent_net_broadcast_untagged(struct kobj_uevent_env *env,
>  		if (!netlink_has_listeners(uevent_sock, 1))
>  			continue;
>  
> -		if (!skb) {
> -			retval = -ENOMEM;
> -			skb = alloc_uevent_skb(env, action_string, devpath);
> -			if (!skb)
> -				continue;
> -		}
> -
>  		retval = netlink_broadcast(uevent_sock, skb_get(skb), 0, 1,
> -					   GFP_KERNEL);
> +					   GFP_NOWAIT);

This all feels very odd.  Again, what has changed to suddenly need this?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] kobject: avoid blocking allocation while holding uevent_sock_mutex
  2026-08-26  7:01 ` Greg KH
@ 2026-08-26  7:17   ` Tao Yu
  2026-08-26  7:28     ` Greg KH
  0 siblings, 1 reply; 4+ messages in thread
From: Tao Yu @ 2026-08-26  7:17 UTC (permalink / raw)
  To: gregkh
  Cc: akpm, dakr, driver-core, linux-kernel, rafael,
	syzbot+c22bb42560ec86726aba, tao1.yu

Hi Greg,

Thanks for the review.

Nothing changed in USB enumeration itself. What syzbot appears to be
hitting is a long-standing latency problem in the untagged uevent path,
not a recent USB-specific regression.

The blocked task is in:

  usb_set_configuration()
    -> device_add()
    -> kobject_uevent_env()
    -> uevent_net_broadcast_untagged()

so the contention point is the global untagged-uevent broadcast path in
lib/kobject_uevent.c.

Looking at the history, the locked allocation/broadcast pattern appears
to date back to 16dff336b33d ("kobject: add kobject_uevent_net_broadcast()").
Because of that, I did not add a Fixes tag or cc stable in v1. I do not
have evidence that this is a recent regression, so I would rather not
add those tags without a stronger justification.

You are also right about the race in my listener pre-check. A listener
can appear after the first unlock, so that part of v1 is not correct.
I will drop that approach in the next revision.

I also should have included the syzbot link and testing details. The
report is here:
https://syzkaller.appspot.com/bug?extid=c22bb42560ec86726aba

syzbot has now tested the proposed patch and reported that the
reproducer did not trigger the issue:
https://groups.google.com/g/syzkaller-bugs/c/q6wmyA-hG6E/m/rLDsbLC-AQAJ

So the patch now has:
Tested-by: syzbot+c22bb42560ec86726aba@syzkaller.appspotmail.com

I also compile-tested it with:
  make lib/kobject_uevent.o

Given the race in v1, I will respin this as a narrower fix and include
the proper Link/Tested-by information.

Thanks,
Tao

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] kobject: avoid blocking allocation while holding uevent_sock_mutex
  2026-08-26  7:17   ` Tao Yu
@ 2026-08-26  7:28     ` Greg KH
  0 siblings, 0 replies; 4+ messages in thread
From: Greg KH @ 2026-08-26  7:28 UTC (permalink / raw)
  To: Tao Yu
  Cc: akpm, dakr, driver-core, linux-kernel, rafael,
	syzbot+c22bb42560ec86726aba

A: http://en.wikipedia.org/wiki/Top_post
Q: Were do I find info about this thing called top-posting?
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

A: No.
Q: Should I include quotations after my reply?

http://daringfireball.net/2007/07/on_top

On Wed, Aug 26, 2026 at 03:17:58PM +0800, Tao Yu wrote:
> Hi Greg,
> 
> Thanks for the review.
> 
> Nothing changed in USB enumeration itself. What syzbot appears to be
> hitting is a long-standing latency problem in the untagged uevent path,
> not a recent USB-specific regression.
> 
> The blocked task is in:
> 
>   usb_set_configuration()
>     -> device_add()
>     -> kobject_uevent_env()
>     -> uevent_net_broadcast_untagged()
> 
> so the contention point is the global untagged-uevent broadcast path in
> lib/kobject_uevent.c.
> 
> Looking at the history, the locked allocation/broadcast pattern appears
> to date back to 16dff336b33d ("kobject: add kobject_uevent_net_broadcast()").
> Because of that, I did not add a Fixes tag or cc stable in v1. I do not
> have evidence that this is a recent regression, so I would rather not
> add those tags without a stronger justification.

But if this is something that userspace can trigger, as syzbot is
showing, then it should be backported to resolve the problem for people
hitting it like yourself.

> You are also right about the race in my listener pre-check. A listener
> can appear after the first unlock, so that part of v1 is not correct.
> I will drop that approach in the next revision.
> 
> I also should have included the syzbot link and testing details. The
> report is here:
> https://syzkaller.appspot.com/bug?extid=c22bb42560ec86726aba
> 
> syzbot has now tested the proposed patch and reported that the
> reproducer did not trigger the issue:
> https://groups.google.com/g/syzkaller-bugs/c/q6wmyA-hG6E/m/rLDsbLC-AQAJ
> 
> So the patch now has:
> Tested-by: syzbot+c22bb42560ec86726aba@syzkaller.appspotmail.com
> 
> I also compile-tested it with:
>   make lib/kobject_uevent.o

But that doesn't actually test things locally.  Please do so on your own
machine as obviously this is a path you can validate yourself.

thanks,

greg k-hh

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-08-26  7:30 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-26  6:46 [PATCH] kobject: avoid blocking allocation while holding uevent_sock_mutex Tao Yu
2026-08-26  7:01 ` Greg KH
2026-08-26  7:17   ` Tao Yu
2026-08-26  7:28     ` Greg KH

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.