2010-11-09 20:43:11

by Pekka Enberg

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

On Tue, Oct 26, 2010 at 2:58 AM, Kees Cook <[email protected]> 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 <[email protected]>
> Signed-off-by: Kees Cook <[email protected]>

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 [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at ?http://www.tux.org/lkml/
>


2010-11-09 20:50:49

by Kees Cook

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

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 <[email protected]> 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 <[email protected]>
> > Signed-off-by: Kees Cook <[email protected]>
>
> 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

2010-11-09 20:51:45

by Pekka Enberg

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

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<[email protected]> 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<[email protected]>
>>> Signed-off-by: Kees Cook<[email protected]>
>> 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 <[email protected]>

Pekka

2010-11-09 21:34:53

by Pekka Enberg

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

On Tue, Nov 9, 2010 at 10:51 PM, Pekka Enberg <[email protected]> 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<[email protected]>
>>> ?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<[email protected]>
>>>> Signed-off-by: Kees Cook<[email protected]>
>>>
>>> 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 <[email protected]>

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?

2010-11-09 21:49:45

by Kees Cook

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

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

2010-11-09 22:54:44

by Linus Torvalds

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

On Tue, Nov 9, 2010 at 12:50 PM, Kees Cook <[email protected]> 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

2010-11-09 23:09:21

by Andrew Morton

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

On Tue, 9 Nov 2010 14:46:30 -0800
Linus Torvalds <[email protected]> wrote:

> On Tue, Nov 9, 2010 at 12:50 PM, Kees Cook <[email protected]> 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.

2010-11-09 23:49:38

by Kees Cook

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

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 <[email protected]> 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 [email protected] 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

2010-11-10 00:55:44

by Linus Torvalds

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

On Tue, Nov 9, 2010 at 3:48 PM, Kees Cook <[email protected]> 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

[email protected]
Al Viro <[email protected]>,
Andrew Morton <[email protected]>,
Jiri Slaby <[email protected]>,
"David S. Miller" <[email protected]>

and I get

Al Viro <[email protected]>
Andrew Morton <[email protected]>
Helge Deller <[email protected]>
David Howells <[email protected]>
Hugh Dickins <[email protected]>
[email protected]

so there is something odd going on there.

Linus

2010-11-10 01:03:35

by Joe Perches

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

On Tue, 2010-11-09 at 16:54 -0800, Linus Torvalds wrote:
> On Tue, Nov 9, 2010 at 3:48 PM, Kees Cook <[email protected]> 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
> [email protected]
> Al Viro <[email protected]>,
> Andrew Morton <[email protected]>,
> Jiri Slaby <[email protected]>,
> "David S. Miller" <[email protected]>
> and I get
> Al Viro <[email protected]>
> Andrew Morton <[email protected]>
> Helge Deller <[email protected]>
> David Howells <[email protected]>
> Hugh Dickins <[email protected]>
> [email protected]
> so there is something odd going on there.

I'd like to know as well.

2010-11-10 02:00:36

by Kees Cook

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

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 <[email protected]> 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
> > [email protected]
> > Al Viro <[email protected]>,
> > Andrew Morton <[email protected]>,
> > Jiri Slaby <[email protected]>,
> > "David S. Miller" <[email protected]>
> > and I get
> > Al Viro <[email protected]>
> > Andrew Morton <[email protected]>
> > Helge Deller <[email protected]>
> > David Howells <[email protected]>
> > Hugh Dickins <[email protected]>
> > [email protected]
> > 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 <[email protected]>
Andrew Morton <[email protected]>
Jiri Slaby <[email protected]>
"Serge E. Hallyn" <[email protected]>
"David S. Miller" <[email protected]>
[email protected]

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

2010-11-11 07:37:05

by Pekka Enberg

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

On Wed, Nov 10, 2010 at 4:00 AM, Kees Cook <[email protected]> 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?