Attachment 255142 Details for Bug 252165 – usb_ethernet Sync ioctl/tick media status changes

[patch] usb_ethernet Sync ioctl/tick media status changes

0001-usb_ethernet-Sync-ioctl-tick-media-status-changes.patch (text/plain), 2.86 KB, created by Ali Abdallah on 2024-11-13 18:03:31 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Ali Abdallah

Created: 2024-11-13 18:03:31 UTC

Size: 2.86 KB

patch

obsolete

>From 39857c0c1d7314226bdfd2d9531fda0a0df8bbe6 Mon Sep 17 00:00:00 2001
>From: Ali Abdallah <ali.abdallah@suse.com>
>Date: Wed, 13 Nov 2024 18:08:59 +0100
>Subject: [PATCH] usb_ethernet Sync ioctl/tick media status changes
>
>By design usbd_do_request_flags releases mtx lock and then acquire it
>later, but this cause the following issue.
>
>1. ue_tick_task acquired the lock
>2. uether ioctl SIOCGIFMEDIA/SIOCSIFMEDIA waiting for the lock
>3. usbd_do_request_flags code called by ue_tick_task releases the lock
>4. uether ioctl acquires the lock, and continues.
>
>The above causes a race in setting mii->mii_media_status and mii_media_active
>which causes the link status to flicker.
>
>link state changed to UP
>link state changed to DOWN
>link state changed to UP
>link state changed to DOWN
>...
>
>see https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=252165
>for more details
>---
> sys/dev/usb/net/usb_ethernet.c | 14 ++++++++++++++
> sys/dev/usb/net/usb_ethernet.h |  5 +++++
> 2 files changed, 19 insertions(+)
>
>diff --git a/sys/dev/usb/net/usb_ethernet.c b/sys/dev/usb/net/usb_ethernet.c
>index 2b90c9249855..15559a2fa90f 100644
>--- a/sys/dev/usb/net/usb_ethernet.c
>+++ b/sys/dev/usb/net/usb_ethernet.c
>@@ -176,6 +176,8 @@ uether_ifattach(struct usb_ether *ue)
> 		goto error;
> 	}
> 
>+	cv_init(&ue->ioctl_cv, "uether_ioctl_cv");
>+
> 	/* fork rest of the attach code */
> 	UE_LOCK(ue);
> 	ue_queue_command(ue, ue_attach_post_task,
>@@ -314,6 +316,8 @@ uether_ifdetach(struct usb_ether *ue)
> 		/* drain any callouts */
> 		usb_callout_drain(&ue->ue_watchdog);
> 
>+		cv_destroy(&ue->ioctl_cv);
>+
> 		/*
> 		 * Detach ethernet first to stop miibus calls from
> 		 * user-space:
>@@ -499,10 +503,14 @@ ue_tick_task(struct usb_proc_msg *_task)
> 	struct usb_ether *ue = task->ue;
> 	if_t ifp = ue->ue_ifp;
> 
>+
> 	if ((if_getdrvflags(ifp) & IFF_DRV_RUNNING) == 0)
> 		return;
> 
>+	ue->ticking = 1;
> 	ue->ue_methods->ue_tick(ue);
>+	cv_signal(&ue->ioctl_cv);
>+	ue->ticking = 0;
> }
> 
> int
>@@ -543,6 +551,12 @@ uether_ioctl(if_t ifp, u_long command, caddr_t data)
> 	case SIOCGIFMEDIA:
> 	case SIOCSIFMEDIA:
> 		if (ue->ue_miibus != NULL) {
>+			/* Wait until tick code yields */
>+			if ((if_getdrvflags(ifp) & IFF_DRV_RUNNING) && ue->ticking) {
>+				UE_LOCK(ue);
>+				cv_wait(&ue->ioctl_cv, ue->ue_mtx);
>+				UE_UNLOCK(ue);
>+			}
> 			mii = device_get_softc(ue->ue_miibus);
> 			error = ifmedia_ioctl(ifp, ifr, &mii->mii_media, command);
> 		} else
>diff --git a/sys/dev/usb/net/usb_ethernet.h b/sys/dev/usb/net/usb_ethernet.h
>index 1f0e121af7a3..cc9367e14d68 100644
>--- a/sys/dev/usb/net/usb_ethernet.h
>+++ b/sys/dev/usb/net/usb_ethernet.h
>@@ -75,6 +75,11 @@ struct usb_ether {
> 	/* NOTE: the "ue_ifp" pointer must be first --hps */
> 	if_t			ue_ifp;
> 	struct mtx		*ue_mtx;
>+
>+	/* Avoid race between ioctl and tick media status */
>+	struct cv		ioctl_cv;
>+	int			ticking;
>+
> 	const struct usb_ether_methods *ue_methods;
> 	struct sysctl_oid	*ue_sysctl_oid;
> 	void			*ue_sc;
>-- 
>2.46.2
>

Actions: View | Diff

Attachments on bug 252165: 221274 | 236901 | 251770 | 255142 | 255162 | 255301