Commit a0456399 authored by Andrzej Pietrasiewicz's avatar Andrzej Pietrasiewicz Committed by Felipe Balbi

usb: gadget: configfs: don't NUL-terminate (sub)compatible ids

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: da424314: "usb: gadget: configfs: OS Extended Compatibility descriptors support"
Reported-by: default avatarDan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: default avatarAndrzej Pietrasiewicz <andrzej.p@samsung.com>
Signed-off-by: default avatarFelipe Balbi <balbi@ti.com>
parent 96e5d312
...@@ -1161,7 +1161,6 @@ static ssize_t interf_grp_compatible_id_store(struct usb_os_desc *desc, ...@@ -1161,7 +1161,6 @@ static ssize_t interf_grp_compatible_id_store(struct usb_os_desc *desc,
if (desc->opts_mutex) if (desc->opts_mutex)
mutex_lock(desc->opts_mutex); mutex_lock(desc->opts_mutex);
memcpy(desc->ext_compat_id, page, l); memcpy(desc->ext_compat_id, page, l);
desc->ext_compat_id[l] = '\0';
if (desc->opts_mutex) if (desc->opts_mutex)
mutex_unlock(desc->opts_mutex); mutex_unlock(desc->opts_mutex);
...@@ -1192,7 +1191,6 @@ static ssize_t interf_grp_sub_compatible_id_store(struct usb_os_desc *desc, ...@@ -1192,7 +1191,6 @@ static ssize_t interf_grp_sub_compatible_id_store(struct usb_os_desc *desc,
if (desc->opts_mutex) if (desc->opts_mutex)
mutex_lock(desc->opts_mutex); mutex_lock(desc->opts_mutex);
memcpy(desc->ext_compat_id + 8, page, l); memcpy(desc->ext_compat_id + 8, page, l);
desc->ext_compat_id[l + 8] = '\0';
if (desc->opts_mutex) if (desc->opts_mutex)
mutex_unlock(desc->opts_mutex); mutex_unlock(desc->opts_mutex);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment