usb: gadget: configfs: Disallow empty function instance name

Quoted 35 lines · Felipe Balbi

Hi,

Quoted 21 lines · Krzysztof Opasiak

Every function should have a type and instance name. Unfortunately in most cases instance name was left unused and unchecked. This may lead to situations like FunctionFS device name identified by "" or some misleading debug messages from TCM like:

tcm: Activating

To avoid this let's add a check that instance name should have at least one character.

Reported-by Stefan Agner <[email protected]>

Signed-off-by Krzysztof Opasiak <[email protected]>

---
  drivers/usb/gadget/configfs.c | 5 +++++
  1 file changed, 5 insertions(+)

drivers/usb/gadget/configfs.c +0 -0

index aeb9f3c40521..bdc9ec597d6a 100644

drivers/usb/gadget/configfs.c static struct config_group *function_make( · line 554

Quoted 7 lines

drivers/usb/gadget/configfs.c +4 -0

@@ -548,6 +548,11 @@ static struct config_group *function_make(
  	*instance_name = '\0';
  	instance_name++;
 
+	if (*instance_name == '\0') {
+		pr_err("Instance name (after .) should not be empty\n");
+		return ERR_PTR(-EINVAL);
+	}

aaaaaand just like that you break potentially existing scripts :-)

Quoted 2 lines

We need to find a better way of enforcing a name which doesn't break existing users.

I'm really open for suggestions how to enforce this without breaking those scripts ;)

The origin of this commit is github issue for libusbgx[1].
So the problem is that library allows to create a function with empty name (because I mistakenly assumed that kernel rejects this) but then it is unable to reinitialize the ConfigFS state because there is a check that disallows this. From my point of view I'd be happy to disallow empty names because it causes some problems (f_fs) or weird debug messages (f_tcm) so is generally inconvenient and seems to be unintentional. But I would like to keep this consistent with kernel policy.