Message ID | 20090826111247.GA79673@clem1.netasq.com |
---|---|
State | Not Applicable, archived |
Delegated to: | David Miller |
Headers | show |
Clement LECIGNE wrote: > Here is a patch that zero the whole sockaddr_at structure before > processing it. It should fix this bug. Same kind of bug at http://lkml.org/lkml/2006/7/25/51 . I wonder why not zero'ing sockaddr at the caller rather than zero'ing at individual ->getname(). Doing so will save some lines. -- 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
Clement LECIGNE a écrit : > Hi, > > In function atalk_getname(), sockaddr_at is returned in userland without > zero'ing the "char sat_zero[8]" field. This bug allows user to display 8 > bytes leaked from the kernel stack. > > Here is a patch that zero the whole sockaddr_at structure before > processing it. It should fix this bug. > > Signed-off-by: Clément Lecigne <clement.lecigne@netasq.com> > --- linux/net/appletalk/ddp.c 2009-08-26 11:35:59.000000000 +0200 > +++ linux/net/appletalk/ddp.c 2009-08-26 11:36:30.000000000 +0200 > @@ -1241,6 +1241,8 @@ static int atalk_getname(struct socket * > if (atalk_autobind(sk) < 0) > return -ENOBUFS; > > + memset(&sat, 0, sizeof(struct sockaddr_at)); > + > *uaddr_len = sizeof(struct sockaddr_at); > > if (peer) { > Hi Clement Well, I submitted same patch some weeks ago and I just checked that it was already in Linus tree. author Eric Dumazet <eric.dumazet@gmail.com> Thu, 6 Aug 2009 02:27:43 +0000 (02:27 +0000) committer David S. Miller <davem@davemloft.net> Thu, 6 Aug 2009 20:08:45 +0000 (13:08 -0700) commit 3d392475c873c10c10d6d96b94d092a34ebd4791 appletalk: fix atalk_getname() leak atalk_getname() can leak 8 bytes of kernel memory to user Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> Signed-off-by: David S. Miller <davem@davemloft.net> Dont worry, it'll be included in upcoming 2.6.31 kernel, and backported to previous ones as well. -- 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
Tetsuo Handa a écrit : > Clement LECIGNE wrote: >> Here is a patch that zero the whole sockaddr_at structure before >> processing it. It should fix this bug. > > Same kind of bug at http://lkml.org/lkml/2006/7/25/51 . > I wonder why not zero'ing sockaddr at the caller rather than > zero'ing at individual ->getname(). Doing so will save some lines. > -- It would not help zeroing buffer before calling ->getname(), since getname() will overwrite it with a memcpy(buffer, &somestruct, sizeof(somestruct)); -- 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
Wed, Aug 26, 2009 at 02:35:57PM +0200, Eric Dumazet wrote: > Clement LECIGNE a écrit : > > Hi, > > > > In function atalk_getname(), sockaddr_at is returned in userland without > > zero'ing the "char sat_zero[8]" field. This bug allows user to display 8 > > bytes leaked from the kernel stack. > > > > Here is a patch that zero the whole sockaddr_at structure before > > processing it. It should fix this bug. > > > > Signed-off-by: Clément Lecigne <clement.lecigne@netasq.com> > > --- linux/net/appletalk/ddp.c 2009-08-26 11:35:59.000000000 +0200 > > +++ linux/net/appletalk/ddp.c 2009-08-26 11:36:30.000000000 +0200 > > @@ -1241,6 +1241,8 @@ static int atalk_getname(struct socket * > > if (atalk_autobind(sk) < 0) > > return -ENOBUFS; > > > > + memset(&sat, 0, sizeof(struct sockaddr_at)); > > + > > *uaddr_len = sizeof(struct sockaddr_at); > > > > if (peer) { > > > Hi Clement > > Well, I submitted same patch some weeks ago and I just checked that > it was already in Linus tree. > > author Eric Dumazet <eric.dumazet@gmail.com> > Thu, 6 Aug 2009 02:27:43 +0000 (02:27 +0000) > committer David S. Miller <davem@davemloft.net> > Thu, 6 Aug 2009 20:08:45 +0000 (13:08 -0700) > commit 3d392475c873c10c10d6d96b94d092a34ebd4791 > > appletalk: fix atalk_getname() leak > > atalk_getname() can leak 8 bytes of kernel memory to user > > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> > Signed-off-by: David S. Miller <davem@davemloft.net> > > > Dont worry, it'll be included in upcoming 2.6.31 kernel, > and backported to previous ones as well. Hi Eric, Oups, shame on me, I have not checked Linus tree before submitting the patch. Sorry,
--- linux/net/appletalk/ddp.c 2009-08-26 11:35:59.000000000 +0200 +++ linux/net/appletalk/ddp.c 2009-08-26 11:36:30.000000000 +0200 @@ -1241,6 +1241,8 @@ static int atalk_getname(struct socket * if (atalk_autobind(sk) < 0) return -ENOBUFS; + memset(&sat, 0, sizeof(struct sockaddr_at)); + *uaddr_len = sizeof(struct sockaddr_at); if (peer) {
Hi, In function atalk_getname(), sockaddr_at is returned in userland without zero'ing the "char sat_zero[8]" field. This bug allows user to display 8 bytes leaked from the kernel stack. Here is a patch that zero the whole sockaddr_at structure before processing it. It should fix this bug. Signed-off-by: Clément Lecigne <clement.lecigne@netasq.com>