net: core: scm: fix information leak to userland

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(-)

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 ?



--
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

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,

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.
--
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