usb: gadget: configfs: Disallow empty function instance name

Quoted 59 lines

Quoted 42 lines · Krzysztof Opasiak

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 ;)

Quoted 8 lines

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.

I think we need to first fix libusbgx to prevent empty names.

I don't want to be the one hearing from Linus that "we don't break userspace". It's clear that empty names shouldn't be allowed, but they _are_ allowed as of today, so how can we cause a regression all of a sudden?

Quoted 1 line

Alan, Greg, any suggestions?

You could do some silly name munging, like changing an empty name to " " whenever you encounter it. Or adding an '_' to the end of any name that consists of nothing but '_' characters.