Skip to content

Commit

Permalink
usb: gadget: configfs: don't NUL-terminate (sub)compatible ids
Browse files Browse the repository at this point in the history
The "Extended Compat ID OS Feature Descriptor Specification" does not
require the (sub)compatible ids to be NUL-terminated, because they
are placed in a fixed-size buffer and only unused parts of it should
contain NULs. If the buffer is fully utilized, there is no place for NULs.

Consequently, the code which uses desc->ext_compat_id never expects the
data contained to be NUL terminated.

If the compatible id is stored after sub-compatible id, and the compatible
id is full length (8 bytes), the (useless) NUL terminator overwrites the
first byte of the sub-compatible id.

If the sub-compatible id is full length (8 bytes), the (useless) NUL
terminator ends up out of the buffer. The situation can happen in the RNDIS
function, where the buffer is a part of struct f_rndis_opts. The next
member of struct f_rndis_opts is a mutex, so its first byte gets
overwritten. The said byte is a part of a mutex'es member which contains
the information on whether the muext is locked or not. This can lead to a
deadlock, because, in a configfs-composed gadget when a function is linked
into a configuration with config_usb_cfg_link(), usb_get_function()
is called, which then calls rndis_alloc(), which tries locking the same
mutex and (wrongly) finds it already locked.

This patch eliminates NUL terminating of the (sub)compatible id.

Cc: <stable@vger.kernel.org> # v3.16+
Fixes: da42431: "usb: gadget: configfs: OS Extended Compatibility descriptors support"
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@samsung.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
  • Loading branch information
Andrzej Pietrasiewicz authored and Felipe Balbi committed Feb 23, 2015
1 parent 96e5d31 commit a045639
Showing 1 changed file with 0 additions and 2 deletions.
2 changes: 0 additions & 2 deletions drivers/usb/gadget/configfs.c
Original file line number Diff line number Diff line change
Expand Up @@ -1161,7 +1161,6 @@ static ssize_t interf_grp_compatible_id_store(struct usb_os_desc *desc,
if (desc->opts_mutex)
mutex_lock(desc->opts_mutex);
memcpy(desc->ext_compat_id, page, l);
desc->ext_compat_id[l] = '\0';

if (desc->opts_mutex)
mutex_unlock(desc->opts_mutex);
Expand Down Expand Up @@ -1192,7 +1191,6 @@ static ssize_t interf_grp_sub_compatible_id_store(struct usb_os_desc *desc,
if (desc->opts_mutex)
mutex_lock(desc->opts_mutex);
memcpy(desc->ext_compat_id + 8, page, l);
desc->ext_compat_id[l + 8] = '\0';

if (desc->opts_mutex)
mutex_unlock(desc->opts_mutex);
Expand Down

0 comments on commit a045639

Please sign in to comment.