2014-04-14 17:11:46

by Kees Cook

Subject: [PATCH] seccomp: fix memory leak on filter attach

This sets the correct error code when final filter memory is unavailable,
and frees the raw filter no matter what.

unreferenced object 0xffff8800d6ea4000 (size 512):
comm "sshd", pid 278, jiffies 4294898315 (age 46.653s)
hex dump (first 32 bytes):
21 00 00 00 04 00 00 00 15 00 01 00 3e 00 00 c0 !...........>...
06 00 00 00 00 00 00 00 21 00 00 00 00 00 00 00 ........!.......
backtrace:
[<ffffffff8151414e>] kmemleak_alloc+0x4e/0xb0
[<ffffffff811a3a40>] __kmalloc+0x280/0x320
[<ffffffff8110842e>] prctl_set_seccomp+0x11e/0x3b0
[<ffffffff8107bb6b>] SyS_prctl+0x3bb/0x4a0
[<ffffffff8152ef2d>] system_call_fastpath+0x1a/0x1f
[<ffffffffffffffff>] 0xffffffffffffffff

Reported-by: Masami Ichikawa <[email protected]>
Signed-off-by: Kees Cook <[email protected]>
Cc: [email protected]
---
kernel/seccomp.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index fd609bd9d6dd..2d7dcdfb7422 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -256,6 +256,7 @@ static long seccomp_attach_filter(struct sock_fprog *fprog)
goto free_prog;

/* Allocate a new seccomp_filter */
+ ret = -ENOMEM;
filter = kzalloc(sizeof(struct seccomp_filter) +
sizeof(struct sock_filter_int) * new_len,
GFP_KERNEL|__GFP_NOWARN);
@@ -265,6 +266,7 @@ static long seccomp_attach_filter(struct sock_fprog *fprog)
ret = sk_convert_filter(fp, fprog->len, filter->insnsi, &new_len);
if (ret)
goto free_filter;
+ kfree(fp);

atomic_set(&filter->usage, 1);
filter->len = new_len;
--
1.7.9.5


--
Kees Cook
Chrome OS Security

2014-04-14 17:17:57

by Alexei Starovoitov

Subject: Re: [PATCH] seccomp: fix memory leak on filter attach

On Mon, Apr 14, 2014 at 10:11 AM, Kees Cook <[email protected]> wrote:
> This sets the correct error code when final filter memory is unavailable,
> and frees the raw filter no matter what.
>
> unreferenced object 0xffff8800d6ea4000 (size 512):
> comm "sshd", pid 278, jiffies 4294898315 (age 46.653s)
> hex dump (first 32 bytes):
> 21 00 00 00 04 00 00 00 15 00 01 00 3e 00 00 c0 !...........>...
> 06 00 00 00 00 00 00 00 21 00 00 00 00 00 00 00 ........!.......
> backtrace:
> [<ffffffff8151414e>] kmemleak_alloc+0x4e/0xb0
> [<ffffffff811a3a40>] __kmalloc+0x280/0x320
> [<ffffffff8110842e>] prctl_set_seccomp+0x11e/0x3b0
> [<ffffffff8107bb6b>] SyS_prctl+0x3bb/0x4a0
> [<ffffffff8152ef2d>] system_call_fastpath+0x1a/0x1f
> [<ffffffffffffffff>] 0xffffffffffffffff
>
> Reported-by: Masami Ichikawa <[email protected]>
> Signed-off-by: Kees Cook <[email protected]>
> Cc: [email protected]

Acked-by: Alexei Starovoitov <[email protected]>

> ---
> kernel/seccomp.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> index fd609bd9d6dd..2d7dcdfb7422 100644
> --- a/kernel/seccomp.c
> +++ b/kernel/seccomp.c
> @@ -256,6 +256,7 @@ static long seccomp_attach_filter(struct sock_fprog *fprog)
> goto free_prog;
>
> /* Allocate a new seccomp_filter */
> + ret = -ENOMEM;
> filter = kzalloc(sizeof(struct seccomp_filter) +
> sizeof(struct sock_filter_int) * new_len,
> GFP_KERNEL|__GFP_NOWARN);
> @@ -265,6 +266,7 @@ static long seccomp_attach_filter(struct sock_fprog *fprog)
> ret = sk_convert_filter(fp, fprog->len, filter->insnsi, &new_len);
> if (ret)
> goto free_filter;
> + kfree(fp);
>
> atomic_set(&filter->usage, 1);
> filter->len = new_len;
> --
> 1.7.9.5
>
>
> --
> Kees Cook
> Chrome OS Security

2014-04-14 17:26:18

by Kees Cook

Subject: Re: [PATCH] seccomp: fix memory leak on filter attach

On Mon, Apr 14, 2014 at 10:17 AM, Alexei Starovoitov <[email protected]> wrote:
> On Mon, Apr 14, 2014 at 10:11 AM, Kees Cook <[email protected]> wrote:
>> This sets the correct error code when final filter memory is unavailable,
>> and frees the raw filter no matter what.
>>
>> unreferenced object 0xffff8800d6ea4000 (size 512):
>> comm "sshd", pid 278, jiffies 4294898315 (age 46.653s)
>> hex dump (first 32 bytes):
>> 21 00 00 00 04 00 00 00 15 00 01 00 3e 00 00 c0 !...........>...
>> 06 00 00 00 00 00 00 00 21 00 00 00 00 00 00 00 ........!.......
>> backtrace:
>> [<ffffffff8151414e>] kmemleak_alloc+0x4e/0xb0
>> [<ffffffff811a3a40>] __kmalloc+0x280/0x320
>> [<ffffffff8110842e>] prctl_set_seccomp+0x11e/0x3b0
>> [<ffffffff8107bb6b>] SyS_prctl+0x3bb/0x4a0
>> [<ffffffff8152ef2d>] system_call_fastpath+0x1a/0x1f
>> [<ffffffffffffffff>] 0xffffffffffffffff
>>
>> Reported-by: Masami Ichikawa <[email protected]>
>> Signed-off-by: Kees Cook <[email protected]>
>> Cc: [email protected]
>
> Acked-by: Alexei Starovoitov <[email protected]>
>
>> ---
>> kernel/seccomp.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
>> index fd609bd9d6dd..2d7dcdfb7422 100644
>> --- a/kernel/seccomp.c
>> +++ b/kernel/seccomp.c
>> @@ -256,6 +256,7 @@ static long seccomp_attach_filter(struct sock_fprog *fprog)
>> goto free_prog;
>>
>> /* Allocate a new seccomp_filter */
>> + ret = -ENOMEM;
>> filter = kzalloc(sizeof(struct seccomp_filter) +
>> sizeof(struct sock_filter_int) * new_len,
>> GFP_KERNEL|__GFP_NOWARN);
>> @@ -265,6 +266,7 @@ static long seccomp_attach_filter(struct sock_fprog *fprog)
>> ret = sk_convert_filter(fp, fprog->len, filter->insnsi, &new_len);
>> if (ret)
>> goto free_filter;
>> + kfree(fp);
>>
>> atomic_set(&filter->usage, 1);
>> filter->len = new_len;
>> --
>> 1.7.9.5
>>
>>
>> --
>> Kees Cook
>> Chrome OS Security

James (or anyone), can you be sure to pull this soon? I'd like to get
it into 3.14 stable ASAP. Under kernel memory pressure, it would be
possible for the prctl to return 0 without having attached the seccomp
filter.

Thanks!

-Kees

--
Kees Cook
Chrome OS Security

2014-04-14 17:35:53

by Alexei Starovoitov

Subject: Re: [PATCH] seccomp: fix memory leak on filter attach

On Mon, Apr 14, 2014 at 10:26 AM, Kees Cook <[email protected]> wrote:
> On Mon, Apr 14, 2014 at 10:17 AM, Alexei Starovoitov <[email protected]> wrote:
>> On Mon, Apr 14, 2014 at 10:11 AM, Kees Cook <[email protected]> wrote:
>>> This sets the correct error code when final filter memory is unavailable,
>>> and frees the raw filter no matter what.
>>>
>>> unreferenced object 0xffff8800d6ea4000 (size 512):
>>> comm "sshd", pid 278, jiffies 4294898315 (age 46.653s)
>>> hex dump (first 32 bytes):
>>> 21 00 00 00 04 00 00 00 15 00 01 00 3e 00 00 c0 !...........>...
>>> 06 00 00 00 00 00 00 00 21 00 00 00 00 00 00 00 ........!.......
>>> backtrace:
>>> [<ffffffff8151414e>] kmemleak_alloc+0x4e/0xb0
>>> [<ffffffff811a3a40>] __kmalloc+0x280/0x320
>>> [<ffffffff8110842e>] prctl_set_seccomp+0x11e/0x3b0
>>> [<ffffffff8107bb6b>] SyS_prctl+0x3bb/0x4a0
>>> [<ffffffff8152ef2d>] system_call_fastpath+0x1a/0x1f
>>> [<ffffffffffffffff>] 0xffffffffffffffff
>>>
>>> Reported-by: Masami Ichikawa <[email protected]>
>>> Signed-off-by: Kees Cook <[email protected]>
>>> Cc: [email protected]
>>
>> Acked-by: Alexei Starovoitov <[email protected]>
>>
>>> ---
>>> kernel/seccomp.c | 2 ++
>>> 1 file changed, 2 insertions(+)
>>>
>>> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
>>> index fd609bd9d6dd..2d7dcdfb7422 100644
>>> --- a/kernel/seccomp.c
>>> +++ b/kernel/seccomp.c
>>> @@ -256,6 +256,7 @@ static long seccomp_attach_filter(struct sock_fprog *fprog)
>>> goto free_prog;
>>>
>>> /* Allocate a new seccomp_filter */
>>> + ret = -ENOMEM;
>>> filter = kzalloc(sizeof(struct seccomp_filter) +
>>> sizeof(struct sock_filter_int) * new_len,
>>> GFP_KERNEL|__GFP_NOWARN);
>>> @@ -265,6 +266,7 @@ static long seccomp_attach_filter(struct sock_fprog *fprog)
>>> ret = sk_convert_filter(fp, fprog->len, filter->insnsi, &new_len);
>>> if (ret)
>>> goto free_filter;
>>> + kfree(fp);
>>>
>>> atomic_set(&filter->usage, 1);
>>> filter->len = new_len;
>>> --
>>> 1.7.9.5
>>>
>>>
>>> --
>>> Kees Cook
>>> Chrome OS Security
>
> James (or anyone), can you be sure to pull this soon? I'd like to get
> it into 3.14 stable ASAP. Under kernel memory pressure, it would be
> possible for the prctl to return 0 without having attached the seccomp
> filter.

I don't think it affects stable.
Fixes: bd4cf0ed331a ("net: filter: rework/optimize internal BPF
interpreter's instruction set")

> Thanks!
>
> -Kees
>
> --
> Kees Cook
> Chrome OS Security

2014-04-14 17:49:44

by Kees Cook

Subject: Re: [PATCH] seccomp: fix memory leak on filter attach

On Mon, Apr 14, 2014 at 10:35 AM, Alexei Starovoitov <[email protected]> wrote:
> On Mon, Apr 14, 2014 at 10:26 AM, Kees Cook <[email protected]> wrote:
>> On Mon, Apr 14, 2014 at 10:17 AM, Alexei Starovoitov <[email protected]> wrote:
>>> On Mon, Apr 14, 2014 at 10:11 AM, Kees Cook <[email protected]> wrote:
>>>> This sets the correct error code when final filter memory is unavailable,
>>>> and frees the raw filter no matter what.
>>>>
>>>> unreferenced object 0xffff8800d6ea4000 (size 512):
>>>> comm "sshd", pid 278, jiffies 4294898315 (age 46.653s)
>>>> hex dump (first 32 bytes):
>>>> 21 00 00 00 04 00 00 00 15 00 01 00 3e 00 00 c0 !...........>...
>>>> 06 00 00 00 00 00 00 00 21 00 00 00 00 00 00 00 ........!.......
>>>> backtrace:
>>>> [<ffffffff8151414e>] kmemleak_alloc+0x4e/0xb0
>>>> [<ffffffff811a3a40>] __kmalloc+0x280/0x320
>>>> [<ffffffff8110842e>] prctl_set_seccomp+0x11e/0x3b0
>>>> [<ffffffff8107bb6b>] SyS_prctl+0x3bb/0x4a0
>>>> [<ffffffff8152ef2d>] system_call_fastpath+0x1a/0x1f
>>>> [<ffffffffffffffff>] 0xffffffffffffffff
>>>>
>>>> Reported-by: Masami Ichikawa <[email protected]>
>>>> Signed-off-by: Kees Cook <[email protected]>
>>>> Cc: [email protected]
>>>
>>> Acked-by: Alexei Starovoitov <[email protected]>
>>>
>>>> ---
>>>> kernel/seccomp.c | 2 ++
>>>> 1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
>>>> index fd609bd9d6dd..2d7dcdfb7422 100644
>>>> --- a/kernel/seccomp.c
>>>> +++ b/kernel/seccomp.c
>>>> @@ -256,6 +256,7 @@ static long seccomp_attach_filter(struct sock_fprog *fprog)
>>>> goto free_prog;
>>>>
>>>> /* Allocate a new seccomp_filter */
>>>> + ret = -ENOMEM;
>>>> filter = kzalloc(sizeof(struct seccomp_filter) +
>>>> sizeof(struct sock_filter_int) * new_len,
>>>> GFP_KERNEL|__GFP_NOWARN);
>>>> @@ -265,6 +266,7 @@ static long seccomp_attach_filter(struct sock_fprog *fprog)
>>>> ret = sk_convert_filter(fp, fprog->len, filter->insnsi, &new_len);
>>>> if (ret)
>>>> goto free_filter;
>>>> + kfree(fp);
>>>>
>>>> atomic_set(&filter->usage, 1);
>>>> filter->len = new_len;
>>>> --
>>>> 1.7.9.5
>>>>
>>>>
>>>> --
>>>> Kees Cook
>>>> Chrome OS Security
>>
>> James (or anyone), can you be sure to pull this soon? I'd like to get
>> it into 3.14 stable ASAP. Under kernel memory pressure, it would be
>> possible for the prctl to return 0 without having attached the seccomp
>> filter.
>
> I don't think it affects stable.
> Fixes: bd4cf0ed331a ("net: filter: rework/optimize internal BPF
> interpreter's instruction set")

Oh! You're completely right. I've been running -next too long. :)
Okay, no urgency at all and the Cc can be dropped. Whew!

-Kees

--
Kees Cook
Chrome OS Security

2014-04-14 18:24:14

by Daniel Borkmann

Subject: Re: [PATCH] seccomp: fix memory leak on filter attach

On 04/14/2014 07:11 PM, Kees Cook wrote:
> This sets the correct error code when final filter memory is unavailable,
> and frees the raw filter no matter what.
>
> unreferenced object 0xffff8800d6ea4000 (size 512):
> comm "sshd", pid 278, jiffies 4294898315 (age 46.653s)
> hex dump (first 32 bytes):
> 21 00 00 00 04 00 00 00 15 00 01 00 3e 00 00 c0 !...........>...
> 06 00 00 00 00 00 00 00 21 00 00 00 00 00 00 00 ........!.......
> backtrace:
> [<ffffffff8151414e>] kmemleak_alloc+0x4e/0xb0
> [<ffffffff811a3a40>] __kmalloc+0x280/0x320
> [<ffffffff8110842e>] prctl_set_seccomp+0x11e/0x3b0
> [<ffffffff8107bb6b>] SyS_prctl+0x3bb/0x4a0
> [<ffffffff8152ef2d>] system_call_fastpath+0x1a/0x1f
> [<ffffffffffffffff>] 0xffffffffffffffff
>
> Reported-by: Masami Ichikawa <[email protected]>
> Signed-off-by: Kees Cook <[email protected]>
> Cc: [email protected]

Acked-by: Daniel Borkmann <[email protected]>

2014-04-14 23:30:10

by Masami Ichikawa

Subject: Re: [PATCH] seccomp: fix memory leak on filter attach

On Tue, Apr 15, 2014 at 2:11 AM, Kees Cook <[email protected]> wrote:
> This sets the correct error code when final filter memory is unavailable,
> and frees the raw filter no matter what.
>
> unreferenced object 0xffff8800d6ea4000 (size 512):
> comm "sshd", pid 278, jiffies 4294898315 (age 46.653s)
> hex dump (first 32 bytes):
> 21 00 00 00 04 00 00 00 15 00 01 00 3e 00 00 c0 !...........>...
> 06 00 00 00 00 00 00 00 21 00 00 00 00 00 00 00 ........!.......
> backtrace:
> [<ffffffff8151414e>] kmemleak_alloc+0x4e/0xb0
> [<ffffffff811a3a40>] __kmalloc+0x280/0x320
> [<ffffffff8110842e>] prctl_set_seccomp+0x11e/0x3b0
> [<ffffffff8107bb6b>] SyS_prctl+0x3bb/0x4a0
> [<ffffffff8152ef2d>] system_call_fastpath+0x1a/0x1f
> [<ffffffffffffffff>] 0xffffffffffffffff
>
> Reported-by: Masami Ichikawa <[email protected]>
> Signed-off-by: Kees Cook <[email protected]>
> Cc: [email protected]

Tested-by: Masami Ichikawa <[email protected]>

> ---
> kernel/seccomp.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> index fd609bd9d6dd..2d7dcdfb7422 100644
> --- a/kernel/seccomp.c
> +++ b/kernel/seccomp.c
> @@ -256,6 +256,7 @@ static long seccomp_attach_filter(struct sock_fprog *fprog)
> goto free_prog;
>
> /* Allocate a new seccomp_filter */
> + ret = -ENOMEM;
> filter = kzalloc(sizeof(struct seccomp_filter) +
> sizeof(struct sock_filter_int) * new_len,
> GFP_KERNEL|__GFP_NOWARN);
> @@ -265,6 +266,7 @@ static long seccomp_attach_filter(struct sock_fprog *fprog)
> ret = sk_convert_filter(fp, fprog->len, filter->insnsi, &new_len);
> if (ret)
> goto free_filter;
> + kfree(fp);
>
> atomic_set(&filter->usage, 1);
> filter->len = new_len;
> --
> 1.7.9.5
>
>
> --
> Kees Cook
> Chrome OS Security

--
Masami Ichikawa