[PATCH] ipc: explicitly clear stack memory in user structs

[PATCH] ipc: explicitly clear stack memory in user structs

[Kees Cook]

CVE-2010-4072

The old shm interface will leak a few bytes of stack contents. Explicitly
clear structure using memset instead of C99-style initialization in case
there are ever holes in the packing.

Cc: stable <stable@kernel.org>
Signed-off-by: Kees Cook <kees.cook@canonical.com>
---

This was originally sent as http://lkml.org/lkml/2010/10/6/486 but was
never taken into any tree.

---
 ipc/shm.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/ipc/shm.c b/ipc/shm.c
index 52ed77e..f943b1e 100644
--- a/ipc/shm.c
+++ b/ipc/shm.c
@@ -473,6 +473,7 @@ static inline unsigned long copy_shmid_to_user(void __user *buf, struct shmid64_
 	    {
 		struct shmid_ds out;
 
+		memset(&out, 0, sizeof(out));
 		ipc64_perm_to_ipc_perm(&in->shm_perm, &out.shm_perm);
 		out.shm_segsz	= in->shm_segsz;
 		out.shm_atime	= in->shm_atime;
@@ -524,6 +525,7 @@ static inline unsigned long copy_shminfo_to_user(void __user *buf, struct shminf
 	    {
 		struct shminfo out;
 
+		memset(&out, 0, sizeof(out));
 		if(in->shmmax > INT_MAX)
 			out.shmmax = INT_MAX;
 		else
-- 
1.7.1

-- 
Kees Cook
Ubuntu Security Team

Re: [PATCH] ipc: explicitly clear stack memory in user structs

[Pekka Enberg]

On Tue, Oct 26, 2010 at 2:58 AM, Kees Cook <kees.cook@canonical.com> wrote:
> CVE-2010-4072
>
> The old shm interface will leak a few bytes of stack contents. Explicitly
> clear structure using memset instead of C99-style initialization in case
> there are ever holes in the packing.
>
> Cc: stable <stable@kernel.org>
> Signed-off-by: Kees Cook <kees.cook@canonical.com>

This looks like a genuine bug fix but I don't see this patch in
mainline. Why is that?

> ---
>
> This was originally sent as http://lkml.org/lkml/2010/10/6/486 but was
> never taken into any tree.
>
> ---
>  ipc/shm.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/ipc/shm.c b/ipc/shm.c
> index 52ed77e..f943b1e 100644
> --- a/ipc/shm.c
> +++ b/ipc/shm.c
> @@ -473,6 +473,7 @@ static inline unsigned long copy_shmid_to_user(void __user *buf, struct shmid64_
>            {
>                struct shmid_ds out;
>
> +               memset(&out, 0, sizeof(out));
>                ipc64_perm_to_ipc_perm(&in->shm_perm, &out.shm_perm);
>                out.shm_segsz   = in->shm_segsz;
>                out.shm_atime   = in->shm_atime;
> @@ -524,6 +525,7 @@ static inline unsigned long copy_shminfo_to_user(void __user *buf, struct shminf
>            {
>                struct shminfo out;
>
> +               memset(&out, 0, sizeof(out));
>                if(in->shmmax > INT_MAX)
>                        out.shmmax = INT_MAX;
>                else
> --
> 1.7.1
>
> --
> Kees Cook
> Ubuntu Security Team
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

Re: [PATCH] ipc: explicitly clear stack memory in user structs

[Kees Cook]

Hi Pekka,

On Tue, Nov 09, 2010 at 10:43:07PM +0200, Pekka Enberg wrote:
> On Tue, Oct 26, 2010 at 2:58 AM, Kees Cook <kees.cook@canonical.com> wrote:
> > CVE-2010-4072
> >
> > The old shm interface will leak a few bytes of stack contents. Explicitly
> > clear structure using memset instead of C99-style initialization in case
> > there are ever holes in the packing.
> >
> > Cc: stable <stable@kernel.org>
> > Signed-off-by: Kees Cook <kees.cook@canonical.com>
> 
> This looks like a genuine bug fix but I don't see this patch in
> mainline. Why is that?

No one has committed it. I don't know why; I've sent it a few times now.

-Kees

-- 
Kees Cook
Ubuntu Security Team

Re: [PATCH] ipc: explicitly clear stack memory in user structs

[Pekka Enberg]

On 9.11.2010 22.50, Kees Cook wrote:
> Hi Pekka,
>
> On Tue, Nov 09, 2010 at 10:43:07PM +0200, Pekka Enberg wrote:
>> On Tue, Oct 26, 2010 at 2:58 AM, Kees Cook<kees.cook@canonical.com>  wrote:
>>> CVE-2010-4072
>>>
>>> The old shm interface will leak a few bytes of stack contents. Explicitly
>>> clear structure using memset instead of C99-style initialization in case
>>> there are ever holes in the packing.
>>>
>>> Cc: stable<stable@kernel.org>
>>> Signed-off-by: Kees Cook<kees.cook@canonical.com>
>> This looks like a genuine bug fix but I don't see this patch in
>> mainline. Why is that?
> No one has committed it. I don't know why; I've sent it a few times now.

Andrew, would you mind picking this up and route it to the appropriate 
person if necessary?

Acked-by: Pekka Enberg <penberg@kernel.org>

             Pekka

Re: [PATCH] ipc: explicitly clear stack memory in user structs

[Pekka Enberg]

On Tue, Nov 9, 2010 at 10:51 PM, Pekka Enberg <penberg@kernel.org> wrote:
> On 9.11.2010 22.50, Kees Cook wrote:
>>
>> Hi Pekka,
>>
>> On Tue, Nov 09, 2010 at 10:43:07PM +0200, Pekka Enberg wrote:
>>>
>>> On Tue, Oct 26, 2010 at 2:58 AM, Kees Cook<kees.cook@canonical.com>
>>>  wrote:
>>>>
>>>> CVE-2010-4072
>>>>
>>>> The old shm interface will leak a few bytes of stack contents.
>>>> Explicitly
>>>> clear structure using memset instead of C99-style initialization in case
>>>> there are ever holes in the packing.
>>>>
>>>> Cc: stable<stable@kernel.org>
>>>> Signed-off-by: Kees Cook<kees.cook@canonical.com>
>>>
>>> This looks like a genuine bug fix but I don't see this patch in
>>> mainline. Why is that?
>>
>> No one has committed it. I don't know why; I've sent it a few times now.
>
> Andrew, would you mind picking this up and route it to the appropriate
> person if necessary?
>
> Acked-by: Pekka Enberg <penberg@kernel.org>

Oh, there seems to be a partial fix in mainline:

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=3af54c9bd9e6f14f896aac1bb0e8405ae0bc7a44

Hmmh?

Re: [PATCH] ipc: explicitly clear stack memory in user structs

[Kees Cook]

Hi Pekka,

On Tue, Nov 09, 2010 at 11:34:50PM +0200, Pekka Enberg wrote:
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=3af54c9bd9e6f14f896aac1bb0e8405ae0bc7a44

Yes, it looks like Vasiliy Kulikov's version[0] from Oct 30th went in
instead of my second[1] or third[2] posting of the patches earlier in the
month.

-Kees

[0] http://lkml.org/lkml/2010/10/30/30
[1] http://lkml.org/lkml/2010/10/6/486
[2] http://lkml.org/lkml/2010/10/25/447

-- 
Kees Cook
Ubuntu Security Team

Re: [PATCH] ipc: explicitly clear stack memory in user structs

[Linus Torvalds]

On Tue, Nov 9, 2010 at 12:50 PM, Kees Cook <kees.cook@canonical.com> wrote:
>
> No one has committed it. I don't know why; I've sent it a few times now.

You seem to have sent it just to lkml. At least in this case the patch
itself was not sent to me (only the subsequent replies were), and the
choice of recipients was fairly odd apart from Andrew (who probably
_is_ the right person).

I don't quite see where you got the particular collection of people from.

                   Linus

Re: [PATCH] ipc: explicitly clear stack memory in user structs

[Andrew Morton]

On Tue, 9 Nov 2010 14:46:30 -0800
Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Tue, Nov 9, 2010 at 12:50 PM, Kees Cook <kees.cook@canonical.com> wrote:
> >
> > No one has committed it. I don't know why; I've sent it a few times now.
> 
> You seem to have sent it just to lkml. At least in this case the patch
> itself was not sent to me (only the subsequent replies were), and the
> choice of recipients was fairly odd apart from Andrew (who probably
> _is_ the right person).

It's in my backlog queue.  Waaaay back.  

The -rc1 merging and kermel summit put me way behind (again) and when
I'm way behind, I start to work in reverse order (mainly to avoid
looking at older versions of patches).  And when I'm working in
time-reverse order, things which were sent a long time ago get delayed
even more.

The good news is that the longer I take to merge something, the less
likely it is that I'll actually merge it ;) Either it got shot down or
a new version came out or someone else merged it.

Re: [PATCH] ipc: explicitly clear stack memory in user structs

[Kees Cook]

Hi Linus,

On Tue, Nov 09, 2010 at 02:46:30PM -0800, Linus Torvalds wrote:
> On Tue, Nov 9, 2010 at 12:50 PM, Kees Cook <kees.cook@canonical.com> wrote:
> > No one has committed it. I don't know why; I've sent it a few times now.
> 
> You seem to have sent it just to lkml. At least in this case the patch
> itself was not sent to me (only the subsequent replies were), and the
> choice of recipients was fairly odd apart from Andrew (who probably
> _is_ the right person).

I've been avoiding sending minor security issues like this directly to you
or security@kernel.org since that probably should be a high-priority
channel.

> 
> I don't quite see where you got the particular collection of people from.

I used scripts/get_maintainer.pl

-Kees

-- 
Kees Cook
Ubuntu Security Team

Re: [PATCH] ipc: explicitly clear stack memory in user structs

[Linus Torvalds]

On Tue, Nov 9, 2010 at 3:48 PM, Kees Cook <kees.cook@canonical.com> wrote:
>
>> I don't quite see where you got the particular collection of people from.
>
> I used scripts/get_maintainer.pl

Hmm. I get totally different results. You had

  linux-kernel@vger.kernel.org
  Al Viro <viro@zeniv.linux.org.uk>,
  Andrew Morton <akpm@linux-foundation.org>,
  Jiri Slaby <jslaby@suse.cz>,
  "David S. Miller" <davem@davemloft.net>

and I get

  Al Viro <viro@ZenIV.linux.org.uk>
  Andrew Morton <akpm@linux-foundation.org>
  Helge Deller <deller@gmx.de>
  David Howells <dhowells@redhat.com>
  Hugh Dickins <hughd@google.com>
  linux-kernel@vger.kernel.org

so there is something odd going on there.

                        Linus

Re: [PATCH] ipc: explicitly clear stack memory in user structs

[Joe Perches]

On Tue, 2010-11-09 at 16:54 -0800, Linus Torvalds wrote:
> On Tue, Nov 9, 2010 at 3:48 PM, Kees Cook <kees.cook@canonical.com> wrote:
> >> I don't quite see where you got the particular collection of people from.
> > I used scripts/get_maintainer.pl
> Hmm. I get totally different results. You had
>   linux-kernel@vger.kernel.org
>   Al Viro <viro@zeniv.linux.org.uk>,
>   Andrew Morton <akpm@linux-foundation.org>,
>   Jiri Slaby <jslaby@suse.cz>,
>   "David S. Miller" <davem@davemloft.net>
> and I get
>   Al Viro <viro@ZenIV.linux.org.uk>
>   Andrew Morton <akpm@linux-foundation.org>
>   Helge Deller <deller@gmx.de>
>   David Howells <dhowells@redhat.com>
>   Hugh Dickins <hughd@google.com>
>   linux-kernel@vger.kernel.org
> so there is something odd going on there.

I'd like to know as well.

Re: [PATCH] ipc: explicitly clear stack memory in user structs

[Kees Cook]

On Tue, Nov 09, 2010 at 05:03:30PM -0800, Joe Perches wrote:
> On Tue, 2010-11-09 at 16:54 -0800, Linus Torvalds wrote:
> > On Tue, Nov 9, 2010 at 3:48 PM, Kees Cook <kees.cook@canonical.com> wrote:
> > >> I don't quite see where you got the particular collection of people from.
> > > I used scripts/get_maintainer.pl
> > Hmm. I get totally different results. You had
> >   linux-kernel@vger.kernel.org
> >   Al Viro <viro@zeniv.linux.org.uk>,
> >   Andrew Morton <akpm@linux-foundation.org>,
> >   Jiri Slaby <jslaby@suse.cz>,
> >   "David S. Miller" <davem@davemloft.net>
> > and I get
> >   Al Viro <viro@ZenIV.linux.org.uk>
> >   Andrew Morton <akpm@linux-foundation.org>
> >   Helge Deller <deller@gmx.de>
> >   David Howells <dhowells@redhat.com>
> >   Hugh Dickins <hughd@google.com>
> >   linux-kernel@vger.kernel.org
> > so there is something odd going on there.
> 
> I'd like to know as well.

Well, let's see what happens. If I rewind the tree to roughly
542181d3769d001c59cd17573dd4381e87d215f2 taking a wild guess at where my
tree was when I send the patch, and run it, here's what I get:

$ ./scripts/get_maintainer.pl 0001-ipc-initialize-structure-memory-to-zero-in-shm.patch
Al Viro <viro@zeniv.linux.org.uk>
Andrew Morton <akpm@linux-foundation.org>
Jiri Slaby <jslaby@suse.cz>
"Serge E. Hallyn" <serue@us.ibm.com>
"David S. Miller" <davem@davemloft.net>
linux-kernel@vger.kernel.org

And since I know Serge's address isn't at ibm any more, I dropped it from
Cc, assuming he didn't care about ipc structure fixups.

If I fast-forward to today and run it, I get the same output you do.

-Kees

-- 
Kees Cook
Ubuntu Security Team

Re: [PATCH] ipc: explicitly clear stack memory in user structs

[Pekka Enberg]

On Wed, Nov 10, 2010 at 4:00 AM, Kees Cook <kees.cook@canonical.com> wrote:
> If I fast-forward to today and run it, I get the same output you do.

Your patch had two parts and I only see one of them in mainline. Care
to resubmit?