Re: [PATCH] usb: gadget: configfs: Disallow empty function instance name

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 12/12/2017 02:16 PM, Felipe Balbi wrote:

Hi,

Krzysztof Opasiak <k.opasiak@xxxxxxxxxxx> writes:
On 12/12/2017 01:31 PM, Felipe Balbi wrote:

Hi,

Krzysztof Opasiak <k.opasiak@xxxxxxxxxxx> writes:
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 <stefan@xxxxxxxx>
Signed-off-by: Krzysztof Opasiak <k.opasiak@xxxxxxxxxxx>
---
   drivers/usb/gadget/configfs.c | 5 +++++
   1 file changed, 5 insertions(+)

diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
index aeb9f3c40521..bdc9ec597d6a 100644
--- a/drivers/usb/gadget/configfs.c
+++ b/drivers/usb/gadget/configfs.c
@@ -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 :-)

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.

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

I created PR for this[1]. If anyone here has any objections to this please let me now as soon as possible.

If there is no veto or other solution, I merge this in the end of week.

Footnotes:
1 - https://github.com/libusbgx/libusbgx/pull/20

Best regards,
--
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux