netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] net: core: sock: fix information leak to userland
@ 2010-10-30 14:26 Vasiliy Kulikov
  2010-10-30 14:35 ` Eric Dumazet
  0 siblings, 1 reply; 3+ messages in thread
From: Vasiliy Kulikov @ 2010-10-30 14:26 UTC (permalink / raw)
  To: kernel-janitors
  Cc: David S. Miller, Eric Dumazet, Eric W. Biederman, Herbert Xu,
	Paul E. McKenney, netdev, linux-kernel

"Address" variable might be not fully initialized in sock->ops->get_name().
The only current implementation is get_name(), it leaves some padding
fields of sockaddr_tipc uninitialized.  It leads to leaking of contents
of kernel stack memory.

Signed-off-by: Vasiliy Kulikov <segooon@gmail.com>
---
 Compile tested.

 net/core/sock.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/net/core/sock.c b/net/core/sock.c
index 3eed542..759dd81 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -930,6 +930,7 @@ int sock_getsockopt(struct socket *sock, int level, int optname,
 	{
 		char address[128];
 
+		memset(&address, 0, sizeof(address));
 		if (sock->ops->getname(sock, (struct sockaddr *)address, &lv, 2))
 			return -ENOTCONN;
 		if (lv < len)
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] net: core: sock: fix information leak to userland
  2010-10-30 14:26 [PATCH] net: core: sock: fix information leak to userland Vasiliy Kulikov
@ 2010-10-30 14:35 ` Eric Dumazet
  2010-10-30 14:49   ` Vasiliy Kulikov
  0 siblings, 1 reply; 3+ messages in thread
From: Eric Dumazet @ 2010-10-30 14:35 UTC (permalink / raw)
  To: Vasiliy Kulikov
  Cc: kernel-janitors, David S. Miller, Eric W. Biederman, Herbert Xu,
	Paul E. McKenney, netdev, linux-kernel

Le samedi 30 octobre 2010 à 18:26 +0400, Vasiliy Kulikov a écrit :
> "Address" variable might be not fully initialized in sock->ops->get_name().
> The only current implementation is get_name(), it leaves some padding
> fields of sockaddr_tipc uninitialized.  It leads to leaking of contents
> of kernel stack memory.
> 
> Signed-off-by: Vasiliy Kulikov <segooon@gmail.com>
> ---
>  Compile tested.
> 
>  net/core/sock.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/net/core/sock.c b/net/core/sock.c
> index 3eed542..759dd81 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -930,6 +930,7 @@ int sock_getsockopt(struct socket *sock, int level, int optname,
>  	{
>  		char address[128];
>  
> +		memset(&address, 0, sizeof(address));
>  		if (sock->ops->getname(sock, (struct sockaddr *)address, &lv, 2))
>  			return -ENOTCONN;
>  		if (lv < len)

???

Please fix the real bug.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] net: core: sock: fix information leak to userland
  2010-10-30 14:35 ` Eric Dumazet
@ 2010-10-30 14:49   ` Vasiliy Kulikov
  0 siblings, 0 replies; 3+ messages in thread
From: Vasiliy Kulikov @ 2010-10-30 14:49 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: kernel-janitors, David S. Miller, Eric W. Biederman, Herbert Xu,
	Paul E. McKenney, netdev, linux-kernel

On Sat, Oct 30, 2010 at 16:35 +0200, Eric Dumazet wrote:
> Le samedi 30 octobre 2010 à 18:26 +0400, Vasiliy Kulikov a écrit :
> > "Address" variable might be not fully initialized in sock->ops->get_name().
> > The only current implementation is get_name(), it leaves some padding
> > fields of sockaddr_tipc uninitialized.  It leads to leaking of contents
> > of kernel stack memory.
> > 
> > Signed-off-by: Vasiliy Kulikov <segooon@gmail.com>
> > ---
> >  Compile tested.
> > 
> >  net/core/sock.c |    1 +
> >  1 files changed, 1 insertions(+), 0 deletions(-)
> > 
> > diff --git a/net/core/sock.c b/net/core/sock.c
> > index 3eed542..759dd81 100644
> > --- a/net/core/sock.c
> > +++ b/net/core/sock.c
> > @@ -930,6 +930,7 @@ int sock_getsockopt(struct socket *sock, int level, int optname,
> >  	{
> >  		char address[128];
> >  
> > +		memset(&address, 0, sizeof(address));
> >  		if (sock->ops->getname(sock, (struct sockaddr *)address, &lv, 2))
> >  			return -ENOTCONN;
> >  		if (lv < len)
> 
> ???
> 
> Please fix the real bug.

What if somebody want to create his own implementation of getname()?
IMO it's much safer to introduce memset() here and relax getname()'s
responsibilities.  Quite many drivers "forget" to initialize outputs
structures.  E.g. new net_device's private field is kzalloc'ed to
simplify driver's code.

-- 
Vasiliy

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2010-10-30 14:49 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-10-30 14:26 [PATCH] net: core: sock: fix information leak to userland Vasiliy Kulikov
2010-10-30 14:35 ` Eric Dumazet
2010-10-30 14:49   ` Vasiliy Kulikov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).