Patchwork can-bcm: fix minor heap overflow

login
register
mail settings
Submitter Oliver Hartkopp
Date Nov. 10, 2010, 10:10 p.m.
Message ID <4CDB1856.4040001@hartkopp.net>
Download mbox | patch
Permalink /patch/70695/
State Accepted
Delegated to: David Miller
Headers show

Comments

Oliver Hartkopp - Nov. 10, 2010, 10:10 p.m.
On 64-bit platforms the ASCII representation of a pointer may be up to 17
bytes long. This patch increases the length of the buffer accordingly.

http://marc.info/?l=linux-netdev&m=128872251418192&w=2

Reported-by: Dan Rosenberg <drosenberg@vsecurity.com>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
CC: Linus Torvalds <torvalds@linux-foundation.org>

---


--
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
Simon Horman - Nov. 12, 2010, 2:39 a.m.
On Wed, Nov 10, 2010 at 11:10:30PM +0100, Oliver Hartkopp wrote:
> On 64-bit platforms the ASCII representation of a pointer may be up to 17
> bytes long. This patch increases the length of the buffer accordingly.
> 
> http://marc.info/?l=linux-netdev&m=128872251418192&w=2
> 
> Reported-by: Dan Rosenberg <drosenberg@vsecurity.com>
> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
> CC: Linus Torvalds <torvalds@linux-foundation.org>
> 
> ---
> 
> diff --git a/net/can/bcm.c b/net/can/bcm.c
> index 08ffe9e..6faa825 100644
> --- a/net/can/bcm.c
> +++ b/net/can/bcm.c
> @@ -125,7 +125,7 @@ struct bcm_sock {
>  	struct list_head tx_ops;
>  	unsigned long dropped_usr_msgs;
>  	struct proc_dir_entry *bcm_proc_read;
> -	char procname [9]; /* pointer printed in ASCII with \0 */
> +	char procname [20]; /* pointer printed in ASCII with \0 */
>  };

If the string may be up to 17 bytes long why are you allocating 20?

>  static inline struct bcm_sock *bcm_sk(const struct sock *sk)
--
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
Dan Rosenberg - Nov. 12, 2010, 2:43 a.m.
> 
> If the string may be up to 17 bytes long why are you allocating 20?
> 

In Oliver's defense, this doesn't matter even a little bit.  The
structure will be allocated with kmalloc-1024 either way.

-Dan

--
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
Simon Horman - Nov. 12, 2010, 2:48 a.m.
On Thu, Nov 11, 2010 at 09:43:58PM -0500, Dan Rosenberg wrote:
> 
> > 
> > If the string may be up to 17 bytes long why are you allocating 20?
> > 
> 
> In Oliver's defense, this doesn't matter even a little bit.  The
> structure will be allocated with kmalloc-1024 either way.

I agree that its very unlikely to make any difference.
I am just curious.
--
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 - Nov. 12, 2010, 10:07 p.m.
From: Oliver Hartkopp <socketcan@hartkopp.net>
Date: Wed, 10 Nov 2010 23:10:30 +0100

> On 64-bit platforms the ASCII representation of a pointer may be up to 17
> bytes long. This patch increases the length of the buffer accordingly.
> 
> http://marc.info/?l=linux-netdev&m=128872251418192&w=2
> 
> Reported-by: Dan Rosenberg <drosenberg@vsecurity.com>
> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
> CC: Linus Torvalds <torvalds@linux-foundation.org>

Patch applied, thanks.
--
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

Patch

diff --git a/net/can/bcm.c b/net/can/bcm.c
index 08ffe9e..6faa825 100644
--- a/net/can/bcm.c
+++ b/net/can/bcm.c
@@ -125,7 +125,7 @@  struct bcm_sock {
 	struct list_head tx_ops;
 	unsigned long dropped_usr_msgs;
 	struct proc_dir_entry *bcm_proc_read;
-	char procname [9]; /* pointer printed in ASCII with \0 */
+	char procname [20]; /* pointer printed in ASCII with \0 */
 };

 static inline struct bcm_sock *bcm_sk(const struct sock *sk)