[PATCH] net: core: scm: fix information leak to userland

[PATCH] net: core: scm: fix information leak to userland

[Vasiliy Kulikov]

Structure cmsghdr is copied to userland with padding bytes
unitialized on architectures where __kernel_size_t is unsigned long.
It leads to leaking of contents of kernel stack memory.

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

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

diff --git a/net/core/scm.c b/net/core/scm.c
index 413cab8..a4a9b70 100644
--- a/net/core/scm.c
+++ b/net/core/scm.c
@@ -233,6 +233,7 @@ int put_cmsg(struct msghdr * msg, int level, int type, int len, void *data)
 		msg->msg_flags |= MSG_CTRUNC;
 		cmlen = msg->msg_controllen;
 	}
+	memset(&cmhdr, 0, sizeof(cmhdr));
 	cmhdr.cmsg_level = level;
 	cmhdr.cmsg_type = type;
 	cmhdr.cmsg_len = cmlen;
-- 
1.7.0.4

Re: [PATCH] net: core: scm: fix information leak to userland

[Eric Dumazet]

Le samedi 30 octobre 2010 à 18:26 +0400, Vasiliy Kulikov a écrit :
> Structure cmsghdr is copied to userland with padding bytes
> unitialized on architectures where __kernel_size_t is unsigned long.
> It leads to leaking of contents of kernel stack memory.
> 
> Signed-off-by: Vasiliy Kulikov <segooon@gmail.com>
> ---
>  Compile tested.
> 
>  net/core/scm.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/net/core/scm.c b/net/core/scm.c
> index 413cab8..a4a9b70 100644
> --- a/net/core/scm.c
> +++ b/net/core/scm.c
> @@ -233,6 +233,7 @@ int put_cmsg(struct msghdr * msg, int level, int type, int len, void *data)
>  		msg->msg_flags |= MSG_CTRUNC;
>  		cmlen = msg->msg_controllen;
>  	}
> +	memset(&cmhdr, 0, sizeof(cmhdr));
>  	cmhdr.cmsg_level = level;
>  	cmhdr.cmsg_type = type;
>  	cmhdr.cmsg_len = cmlen;


???

struct cmsghdr {
        __kernel_size_t cmsg_len;       /* data byte count, including hdr */
        int             cmsg_level;     /* originating protocol */
        int             cmsg_type;      /* protocol-specific type */
};

Could you explain where are the padding bytes ?

Re: [PATCH] net: core: scm: fix information leak to userland

[Vasiliy Kulikov]

On Sat, Oct 30, 2010 at 16:33 +0200, Eric Dumazet wrote:
> Le samedi 30 octobre 2010 à 18:26 +0400, Vasiliy Kulikov a écrit :
> > Structure cmsghdr is copied to userland with padding bytes
> > unitialized on architectures where __kernel_size_t is unsigned long.
> > It leads to leaking of contents of kernel stack memory.
> > 
> > Signed-off-by: Vasiliy Kulikov <segooon@gmail.com>
> > ---
> >  Compile tested.
> > 
> >  net/core/scm.c |    1 +
> >  1 files changed, 1 insertions(+), 0 deletions(-)
> > 
> > diff --git a/net/core/scm.c b/net/core/scm.c
> > index 413cab8..a4a9b70 100644
> > --- a/net/core/scm.c
> > +++ b/net/core/scm.c
> > @@ -233,6 +233,7 @@ int put_cmsg(struct msghdr * msg, int level, int type, int len, void *data)
> >  		msg->msg_flags |= MSG_CTRUNC;
> >  		cmlen = msg->msg_controllen;
> >  	}
> > +	memset(&cmhdr, 0, sizeof(cmhdr));
> >  	cmhdr.cmsg_level = level;
> >  	cmhdr.cmsg_type = type;
> >  	cmhdr.cmsg_len = cmlen;
> 
> 
> ???
> 
> struct cmsghdr {
>         __kernel_size_t cmsg_len;       /* data byte count, including hdr */
>         int             cmsg_level;     /* originating protocol */
>         int             cmsg_type;      /* protocol-specific type */
> };
> 
> Could you explain where are the padding bytes ?

Ah, sorry, nowhere :)  int is stored quite OK after long.  Please ignore this patch.

Thanks,

-- 
Vasiliy

Re: [PATCH] net: core: scm: fix information leak to userland

[David Miller]

Your patches are almost entirely baseless.

You haven't even made an effort to show a real case, in detail,
where your patches actually fix a bug.  The CMSG case shows
that you didn't even bother to look at the assembly of even
one architecture to see if padding bytes even existed in the
structure, and that furthermore even if they existed that they
would leak out ever.

I don't even buy the "preventative nature" argument for the
address[128] thing.  If a protocol is leaking kernel memory in that
case, it also isn't filling in the address value properly, which is a
bug times two.

I absolutely am not applying these patches, sorry.