Hi Greg,
On 1/25/23 8:09 PM, Greg Kroah-Hartman wrote:
On Wed, Jan 25, 2023 at 12:27:40PM +0530, Udipto Goswami wrote:
During enumeration or composition switch, if a userspace process
agnostic of the conventions of configs tries to create function symlink
seven after the UDC is bound to current config which is not correct.
Potentially it can create duplicates within the current config.
I'm sorry, but I can not parse this paragraph at all.
Exactly what is the problem here? Is userspace doing something it
shouldn't? If so, fix userspace, right?
Prevent this by adding a check if udc_name already exists then bail
out of cfg_link.
Why? What will this help prevent if userspace already messed things up?
confused,
Apologies about this, had already pushed a v2 fixing the commit text[1]
It's not particularly any userspace process doing something wrong, but
even in general usage if a end users is able to execute commands
directly through command line accessing kernel file system and gets
access to config/usb_gadget/g1/, one an easily create
a duplicate symlink for an existing function.
Step1:
ln -s X1 ffs.a
-->cfg_link
--> usb_get_function(ffs.a)
->ffs_alloc
CFG->FUNC_LIST: <ffs.a>
C->FUNCTION: <empty>
Step2:
echo udc.name > /config/usb_gadget/g1/UDC
--> UDC_store
->composite_bind
->usb_add_function
CFG->FUNC_LIST: <empty>
C->FUNCTION: <ffs.a>
Step3:
ln -s Y1 ffs.a
-->cfg_link
-->usb_get_function(ffs.a)
->ffs_alloc
CFG->FUNC_LIST: <ffs.a>
C->FUNCTION: <ffs.a>
both the lists corresponds to the same function instance ffs.a but the
usb_function* pointer is different because in step 3 ffs_alloc has
created a new reference to usb_function* for ffs.a and added it to cfg_list.
Step4:
Now a composition switch involving <ffs.b,ffs.a> is executed.
the composition switch will involve 3 things:
1. unlinking the previous functions existing
2. creating new symlinks
3. writing UDC
However, the composition switch is generally taken care by a userspace
process which creates the symlinks in its own nomenclature(X*) and
removes only those. So it won't be able to remove Y1 which user had
created by own.
Due to this the new symlinks cannot be created for ffs.a since the entry
already exists in CFG->FUNC_LIST.
The state of the CFG->FUNC_LIST is as follows:
CFG->FUNC_LIST: <ffs.a>
Since UDC is binded already, creating these dummy/incorrect symlinks,
that can interfere with successive enumeration attempts can be avoided.
Cfg->func_list points to the list of functions that corresponds to that
particular configuration. C->function points to the list of functions
that are already bound to UDC and enumerated successfully.Ideally, when
a particular configuration is already enumerated and bounded, meddling
with cfg->func_list can create inconsistencies.
[1]:
https://lore.kernel.org/all/20230125072138.21925-1-quic_ugoswami@xxxxxxxxxxx/
Thanks,
-Udipto
