linux-kernel - [PATCH 3.16.y-ckt 051/165] usb: gadget: configfs: don't NUL-terminate (sub)compatible ids

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <1427292138-7021-52-git-send-email-luis.henriques@canonical.com>
Date:	Wed, 25 Mar 2015 14:00:24 +0000
From:	Luis Henriques <luis.henriques@...onical.com>
To:	linux-kernel@...r.kernel.org, stable@...r.kernel.org,
	kernel-team@...ts.ubuntu.com
Cc:	Andrzej Pietrasiewicz <andrzej.p@...sung.com>,
	Felipe Balbi <balbi@...com>,
	Luis Henriques <luis.henriques@...onical.com>
Subject: [PATCH 3.16.y-ckt 051/165] usb: gadget: configfs: don't NUL-terminate (sub)compatible ids

3.16.7-ckt9 -stable review patch.  If anyone has any objections, please let me know.

------------------

From: Andrzej Pietrasiewicz <andrzej.p@...sung.com>

commit a0456399fb07155637a2b597b91cc1c63bc25141 upstream.

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.

Fixes: da4243145fb1: "usb: gadget: configfs: OS Extended Compatibility descriptors support"
Reported-by: Dan Carpenter <dan.carpenter@...cle.com>
Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@...sung.com>
Signed-off-by: Felipe Balbi <balbi@...com>
Signed-off-by: Luis Henriques <luis.henriques@...onical.com>
---
 drivers/usb/gadget/configfs.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
index 97142146eead..45a94a77d986 100644
--- a/drivers/usb/gadget/configfs.c
+++ b/drivers/usb/gadget/configfs.c
@@ -1163,7 +1163,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);
@@ -1194,7 +1193,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);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/