diff mbox

caif: add a sanity check to the tty name

Message ID 20130903090232.GA4351@elgon.mountain
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Dan Carpenter Sept. 3, 2013, 9:02 a.m. UTC
"tty->name" and "name" are a 64 character buffers.  My static checker
complains because we add the "cf" on the front so it look like we are
copying a 66 character string into a 64 character buffer.

Also if the name is larger than IFNAMSIZ (16) it triggers a BUG_ON()
inside the call to alloc_netdev().

This is all under CAP_SYS_ADMIN so it's not a security fix, it just adds
a little robustness.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Dmitry Tarnyagin Sept. 3, 2013, 10:47 a.m. UTC | #1
Hi Dan,

Agree, thank you!

With best regards,
Dmitry

Den 03. sep. 2013 11:02, skrev Dan Carpenter:
> -	sprintf(name, "cf%s", tty->name);
> +	result = snprintf(name, sizeof(name), "cf%s", tty->name);
> +	if (result >= IFNAMSIZ)
> +		return -EINVAL;

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller Sept. 4, 2013, 4:54 a.m. UTC | #2
From: Dan Carpenter <dan.carpenter@oracle.com>
Date: Tue, 3 Sep 2013 12:02:32 +0300

> "tty->name" and "name" are a 64 character buffers.  My static checker
> complains because we add the "cf" on the front so it look like we are
> copying a 66 character string into a 64 character buffer.
> 
> Also if the name is larger than IFNAMSIZ (16) it triggers a BUG_ON()
> inside the call to alloc_netdev().
> 
> This is all under CAP_SYS_ADMIN so it's not a security fix, it just adds
> a little robustness.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Applied.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/net/caif/caif_serial.c b/drivers/net/caif/caif_serial.c
index 34dea95..88a6a58 100644
--- a/drivers/net/caif/caif_serial.c
+++ b/drivers/net/caif/caif_serial.c
@@ -347,7 +347,9 @@  static int ldisc_open(struct tty_struct *tty)
 	/* release devices to avoid name collision */
 	ser_release(NULL);
 
-	sprintf(name, "cf%s", tty->name);
+	result = snprintf(name, sizeof(name), "cf%s", tty->name);
+	if (result >= IFNAMSIZ)
+		return -EINVAL;
 	dev = alloc_netdev(sizeof(*ser), name, caifdev_setup);
 	if (!dev)
 		return -ENOMEM;