usb: gadget: f_ecm: Fix ecm_opts->bound logic in bind path

PATCH Superseded Kuen-Han Tsai 2 participants 3 messages Sep 3, 2025 – Sep 4, 2025 USB

lore

Share & tools

lore

Kuen-Han Tsai

author

2025-09-03 08:30 UTC #0 lore raw

PATCH 1/1 usb: gadget: f_ecm: Fix ecm_opts->bound logic in bind path

The bound flag in ecm_opts is being set to true even if gether_register_netdev() failed.

Set ecm_opts->bound to true only upon success.

Fixes d65e6b6e884a ("usb: gadget: f_ecm: Always set current gadget in ecm_bind()")

Cc [email protected]

Signed-off-by Kuen-Han Tsai <[email protected]>

---
 drivers/usb/gadget/function/f_ecm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

drivers/usb/gadget/function/f_ecm.c +2 -1

index 027226325039..9f5ed6f32a62 100644
@@ -690,13 +690,14 @@ ecm_bind(struct usb_configuration *c, struct usb_function *f)
 
 	if (!ecm_opts->bound) {
 		status = gether_register_netdev(ecm_opts->net);
-		ecm_opts->bound = true;
 	}
 
 	mutex_unlock(&ecm_opts->lock);
 	if (status)
 		return status;
 
+	ecm_opts->bound = true;
+
 	ecm_string_defs[1].s = ecm->ethaddr;
 
 	us = usb_gstrings_attach(cdev, ecm_strings,

Signature · 1 line

2.51.0.338.gd7d06c2dae-goog

Krzysztof Kozlowski

2025-09-03 08:33 UTC #1 lore raw

Quoted 18 lines · Kuen-Han Tsai

The bound flag in ecm_opts is being set to true even if gether_register_netdev() failed.
Set ecm_opts->bound to true only upon success.
Fixes d65e6b6e884a ("usb: gadget: f_ecm: Always set current gadget in ecm_bind()")
Cc [email protected]
Signed-off-by Kuen-Han Tsai <[email protected]>
---
 drivers/usb/gadget/function/f_ecm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/usb/gadget/function/f_ecm.c +0 -0
index 027226325039..9f5ed6f32a62 100644
@@ -690,13 +690,14 @@ ecm_bind(struct usb_configuration *c, struct usb_function *f)
 

drivers/usb/gadget/function/f_ecm.c ecm_bind(struct usb_configuration *c, struct usb_function *f) · line 699

Quoted 10 lines

drivers/usb/gadget/function/f_ecm.c +1 -1

 	if (!ecm_opts->bound) {
 		status = gether_register_netdev(ecm_opts->net);
-		ecm_opts->bound = true;
 	}
 
 	mutex_unlock(&ecm_opts->lock);
 	if (status)
 		return status;
 
+	ecm_opts->bound = true;

Now it is outside of mutex, so this is raising questions you should have answered in commit msg.

Best regards,
Krzysztof

Kuen-Han Tsai

author

2025-09-04 05:49 UTC #2 lore raw

Hi Krzysztof,

Quoted 25 lines · Krzysztof Kozlowski

Quoted 23 lines · Kuen-Han Tsai
The bound flag in ecm_opts is being set to true even if gether_register_netdev() failed.
Set ecm_opts->bound to true only upon success.
Fixes d65e6b6e884a ("usb: gadget: f_ecm: Always set current gadget in ecm_bind()")
Cc [email protected]
Signed-off-by Kuen-Han Tsai <[email protected]>
---
 drivers/usb/gadget/function/f_ecm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/usb/gadget/function/f_ecm.c +0 -1
index 027226325039..9f5ed6f32a62 100644
@@ -690,13 +690,14 @@ ecm_bind(struct usb_configuration *c, struct usb_function *f)
 
      if (!ecm_opts->bound) {
              status = gether_register_netdev(ecm_opts->net);
-             ecm_opts->bound = true;
      }
 

Quoted 9 lines

drivers/usb/gadget/function/f_ecm.c ecm_bind(struct usb_configuration *c, struct usb_function *f) · line 699
Quoted 5 lines
drivers/usb/gadget/function/f_ecm.c +1 -0
      mutex_unlock(&ecm_opts->lock);
      if (status)
              return status;
 
+     ecm_opts->bound = true;
Now it is outside of mutex, so this is raising questions you should have answered in commit msg.

Thanks for the review.

1. Commit da92801c647c ("usb: gadget: f_ecm: add configfs support") introduced a mutex_lock in the ecm_bind function, but this lock did not protect the ecm_opts->bound flag. Subsequently, commit d65e6b6e884a ("usb: gadget: f_ecm: Always set current gadget in ecm_bind()") moved the status check outside the locked section but neglected to also move the assignment for ecm_opts->bound.
2. The caller, configfs_composite_bind(), binds functions sequentially, which prevents race conditions when accessing ecm_opts->bound.

I will update the commit message and submit a new version shortly.

Regards,
Kuen-Han