2016-01-29 19:24:38

by Cong Wang

[permalink] [raw]
Subject: [PATCH v2 net] nfc: use GFP_USER for user-controlled kmalloc

These two functions are called in sendmsg path, and the
'len' is passed from user-space, so we should not allow
malicious users to OOM kernel on purpose.

Reported-by: Dmitry Vyukov <[email protected]>
Cc: Lauro Ramos Venancio <[email protected]>
Cc: Aloisio Almeida Jr <[email protected]>
Cc: Samuel Ortiz <[email protected]>
Signed-off-by: Cong Wang <[email protected]>
---
net/nfc/llcp_commands.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/nfc/llcp_commands.c b/net/nfc/llcp_commands.c
index 3621a90..3425532 100644
--- a/net/nfc/llcp_commands.c
+++ b/net/nfc/llcp_commands.c
@@ -663,7 +663,7 @@ int nfc_llcp_send_i_frame(struct nfc_llcp_sock *sock,
return -ENOBUFS;
}

- msg_data = kzalloc(len, GFP_KERNEL);
+ msg_data = kmalloc(len, GFP_USER | __GFP_NOWARN);
if (msg_data == NULL)
return -ENOMEM;

@@ -729,7 +729,7 @@ int nfc_llcp_send_ui_frame(struct nfc_llcp_sock *sock, u8 ssap, u8 dsap,
if (local == NULL)
return -ENODEV;

- msg_data = kzalloc(len, GFP_KERNEL);
+ msg_data = kmalloc(len, GFP_USER | __GFP_NOWARN);
if (msg_data == NULL)
return -ENOMEM;

--
1.8.3.1



2016-01-29 19:46:05

by Eric Dumazet

[permalink] [raw]
Subject: Re: [PATCH v2 net] nfc: use GFP_USER for user-controlled kmalloc

On Fri, 2016-01-29 at 11:24 -0800, Cong Wang wrote:
> These two functions are called in sendmsg path, and the
> 'len' is passed from user-space, so we should not allow
> malicious users to OOM kernel on purpose.
>
> Reported-by: Dmitry Vyukov <[email protected]>
> Cc: Lauro Ramos Venancio <[email protected]>
> Cc: Aloisio Almeida Jr <[email protected]>
> Cc: Samuel Ortiz <[email protected]>
> Signed-off-by: Cong Wang <[email protected]>
> ---

Note that the issue is not OOM the kernel (as the allocation is
attempted even after your patch), but having a way to
spill stack traces in the syslog.

Acked-by: Eric Dumazet <[email protected]>

Thanks!




2016-01-29 22:02:16

by Julian Calaby

[permalink] [raw]
Subject: Re: [PATCH v2 net] nfc: use GFP_USER for user-controlled kmalloc

Hi Cong

On Sat, Jan 30, 2016 at 6:46 AM, Eric Dumazet <[email protected]> wrote:
> On Fri, 2016-01-29 at 11:24 -0800, Cong Wang wrote:
>> These two functions are called in sendmsg path, and the
>> 'len' is passed from user-space, so we should not allow
>> malicious users to OOM kernel on purpose.
>>
>> Reported-by: Dmitry Vyukov <[email protected]>
>> Cc: Lauro Ramos Venancio <[email protected]>
>> Cc: Aloisio Almeida Jr <[email protected]>
>> Cc: Samuel Ortiz <[email protected]>
>> Signed-off-by: Cong Wang <[email protected]>
>> ---
>
> Note that the issue is not OOM the kernel (as the allocation is
> attempted even after your patch), but having a way to
> spill stack traces in the syslog.
>
> Acked-by: Eric Dumazet <[email protected]>

Reviewed-by: Julian Calaby <[email protected]>

Thanks,

--
Julian Calaby

Email: [email protected]
Profile: http://www.google.com/profiles/julian.calaby/

2016-02-24 18:45:24

by David Miller

[permalink] [raw]
Subject: Re: [PATCH v2 net] nfc: use GFP_USER for user-controlled kmalloc

From: Cong Wang <[email protected]>
Date: Wed, 24 Feb 2016 10:41:29 -0800

> On Fri, Jan 29, 2016 at 11:24 AM, Cong Wang <[email protected]> wrote:
>> These two functions are called in sendmsg path, and the
>> 'len' is passed from user-space, so we should not allow
>> malicious users to OOM kernel on purpose.
>>
>> Reported-by: Dmitry Vyukov <[email protected]>
>> Cc: Lauro Ramos Venancio <[email protected]>
>> Cc: Aloisio Almeida Jr <[email protected]>
>> Cc: Samuel Ortiz <[email protected]>
>> Signed-off-by: Cong Wang <[email protected]>
>
> Ping...
>
> David, this patch seems still not applied, I guess you expect NFC
> maintainer to take it, but this doesn't happen. Could you take it?

The NFC maintainer needs to take this, ping them explicitly if
you have to.

Thanks.

2016-02-24 18:52:27

by Samuel Ortiz

[permalink] [raw]
Subject: Re: [PATCH v2 net] nfc: use GFP_USER for user-controlled kmalloc

On Wed, Feb 24, 2016 at 10:41:29AM -0800, Cong Wang wrote:
> On Fri, Jan 29, 2016 at 11:24 AM, Cong Wang <[email protected]> wrote:
> > These two functions are called in sendmsg path, and the
> > 'len' is passed from user-space, so we should not allow
> > malicious users to OOM kernel on purpose.
> >
> > Reported-by: Dmitry Vyukov <[email protected]>
> > Cc: Lauro Ramos Venancio <[email protected]>
> > Cc: Aloisio Almeida Jr <[email protected]>
> > Cc: Samuel Ortiz <[email protected]>
> > Signed-off-by: Cong Wang <[email protected]>
>
> Ping...
>
> David, this patch seems still not applied, I guess you expect NFC
> maintainer to take it, but this doesn't happen. Could you take it?
I'll look at it later today.

Cheers,
Samuel.

2016-02-24 18:41:30

by Cong Wang

[permalink] [raw]
Subject: Re: [PATCH v2 net] nfc: use GFP_USER for user-controlled kmalloc

On Fri, Jan 29, 2016 at 11:24 AM, Cong Wang <[email protected]> wrote:
> These two functions are called in sendmsg path, and the
> 'len' is passed from user-space, so we should not allow
> malicious users to OOM kernel on purpose.
>
> Reported-by: Dmitry Vyukov <[email protected]>
> Cc: Lauro Ramos Venancio <[email protected]>
> Cc: Aloisio Almeida Jr <[email protected]>
> Cc: Samuel Ortiz <[email protected]>
> Signed-off-by: Cong Wang <[email protected]>

Ping...

David, this patch seems still not applied, I guess you expect NFC
maintainer to take it, but this doesn't happen. Could you take it?

(I can resend it if you need.)

Thanks!

> ---
> net/nfc/llcp_commands.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/nfc/llcp_commands.c b/net/nfc/llcp_commands.c
> index 3621a90..3425532 100644
> --- a/net/nfc/llcp_commands.c
> +++ b/net/nfc/llcp_commands.c
> @@ -663,7 +663,7 @@ int nfc_llcp_send_i_frame(struct nfc_llcp_sock *sock,
> return -ENOBUFS;
> }
>
> - msg_data = kzalloc(len, GFP_KERNEL);
> + msg_data = kmalloc(len, GFP_USER | __GFP_NOWARN);
> if (msg_data == NULL)
> return -ENOMEM;
>
> @@ -729,7 +729,7 @@ int nfc_llcp_send_ui_frame(struct nfc_llcp_sock *sock, u8 ssap, u8 dsap,
> if (local == NULL)
> return -ENODEV;
>
> - msg_data = kzalloc(len, GFP_KERNEL);
> + msg_data = kmalloc(len, GFP_USER | __GFP_NOWARN);
> if (msg_data == NULL)
> return -ENOMEM;
>
> --
> 1.8.3.1
>

2016-02-25 07:43:29

by Samuel Ortiz

[permalink] [raw]
Subject: Re: [PATCH v2 net] nfc: use GFP_USER for user-controlled kmalloc

Hi Cong,

On Fri, Jan 29, 2016 at 11:24:24AM -0800, Cong Wang wrote:
> These two functions are called in sendmsg path, and the
> 'len' is passed from user-space, so we should not allow
> malicious users to OOM kernel on purpose.
>
> Reported-by: Dmitry Vyukov <[email protected]>
> Cc: Lauro Ramos Venancio <[email protected]>
> Cc: Aloisio Almeida Jr <[email protected]>
> Cc: Samuel Ortiz <[email protected]>
> Signed-off-by: Cong Wang <[email protected]>
> ---
> net/nfc/llcp_commands.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
Applied to nfc-next, thanks.

Cheers,
Samuel.

2016-02-24 18:53:46

by Cong Wang

[permalink] [raw]
Subject: Re: [PATCH v2 net] nfc: use GFP_USER for user-controlled kmalloc

On Wed, Feb 24, 2016 at 10:45 AM, David Miller <[email protected]> wrote:
> From: Cong Wang <[email protected]>
> Date: Wed, 24 Feb 2016 10:41:29 -0800
>
>> On Fri, Jan 29, 2016 at 11:24 AM, Cong Wang <[email protected]> wrote:
>>> These two functions are called in sendmsg path, and the
>>> 'len' is passed from user-space, so we should not allow
>>> malicious users to OOM kernel on purpose.
>>>
>>> Reported-by: Dmitry Vyukov <[email protected]>
>>> Cc: Lauro Ramos Venancio <[email protected]>
>>> Cc: Aloisio Almeida Jr <[email protected]>
>>> Cc: Samuel Ortiz <[email protected]>
>>> Signed-off-by: Cong Wang <[email protected]>
>>
>> Ping...
>>
>> David, this patch seems still not applied, I guess you expect NFC
>> maintainer to take it, but this doesn't happen. Could you take it?
>
> The NFC maintainer needs to take this, ping them explicitly if
> you have to.

OK. I think Samuel is looking at it now.

Thanks!