All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net] sctp: use __GFP_NOWARN for sctpw.fifo allocation
@ 2017-08-05 12:31 ` Xin Long
  0 siblings, 0 replies; 10+ messages in thread
From: Xin Long @ 2017-08-05 12:31 UTC (permalink / raw)
  To: network dev, linux-sctp; +Cc: davem, Marcelo Ricardo Leitner, Neil Horman

Chen Wei found a kernel call trace when modprobe sctp_probe with
bufsize set with a huge value.

It's because in sctpprobe_init when alloc memory for sctpw.fifo,
the size is got from userspace. If it is too large, kernel will
fail and give a warning.

As there will be a fallback allocation later, this patch is just
to fail silently and return ret, just as commit 0ccc22f425e5
("sit: use __GFP_NOWARN for user controlled allocation") did.

Reported-by: Chen Wei <weichen@redhat.com>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 net/sctp/probe.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/sctp/probe.c b/net/sctp/probe.c
index 6cc2152..5bf3164 100644
--- a/net/sctp/probe.c
+++ b/net/sctp/probe.c
@@ -210,7 +210,7 @@ static __init int sctpprobe_init(void)
 
 	init_waitqueue_head(&sctpw.wait);
 	spin_lock_init(&sctpw.lock);
-	if (kfifo_alloc(&sctpw.fifo, bufsize, GFP_KERNEL))
+	if (kfifo_alloc(&sctpw.fifo, bufsize, GFP_KERNEL | __GFP_NOWARN))
 		return ret;
 
 	if (!proc_create(procname, S_IRUSR, init_net.proc_net,
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH net] sctp: use __GFP_NOWARN for sctpw.fifo allocation
@ 2017-08-05 12:31 ` Xin Long
  0 siblings, 0 replies; 10+ messages in thread
From: Xin Long @ 2017-08-05 12:31 UTC (permalink / raw)
  To: network dev, linux-sctp; +Cc: davem, Marcelo Ricardo Leitner, Neil Horman

Chen Wei found a kernel call trace when modprobe sctp_probe with
bufsize set with a huge value.

It's because in sctpprobe_init when alloc memory for sctpw.fifo,
the size is got from userspace. If it is too large, kernel will
fail and give a warning.

As there will be a fallback allocation later, this patch is just
to fail silently and return ret, just as commit 0ccc22f425e5
("sit: use __GFP_NOWARN for user controlled allocation") did.

Reported-by: Chen Wei <weichen@redhat.com>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 net/sctp/probe.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/sctp/probe.c b/net/sctp/probe.c
index 6cc2152..5bf3164 100644
--- a/net/sctp/probe.c
+++ b/net/sctp/probe.c
@@ -210,7 +210,7 @@ static __init int sctpprobe_init(void)
 
 	init_waitqueue_head(&sctpw.wait);
 	spin_lock_init(&sctpw.lock);
-	if (kfifo_alloc(&sctpw.fifo, bufsize, GFP_KERNEL))
+	if (kfifo_alloc(&sctpw.fifo, bufsize, GFP_KERNEL | __GFP_NOWARN))
 		return ret;
 
 	if (!proc_create(procname, S_IRUSR, init_net.proc_net,
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH net] sctp: use __GFP_NOWARN for sctpw.fifo allocation
  2017-08-05 12:31 ` Xin Long
@ 2017-08-05 17:08   ` Marcelo Ricardo Leitner
  -1 siblings, 0 replies; 10+ messages in thread
From: Marcelo Ricardo Leitner @ 2017-08-05 17:08 UTC (permalink / raw)
  To: Xin Long; +Cc: network dev, linux-sctp, davem, Neil Horman

On Sat, Aug 05, 2017 at 08:31:09PM +0800, Xin Long wrote:
> Chen Wei found a kernel call trace when modprobe sctp_probe with
> bufsize set with a huge value.
> 
> It's because in sctpprobe_init when alloc memory for sctpw.fifo,
> the size is got from userspace. If it is too large, kernel will
> fail and give a warning.

Yes but sctp_probe can only be loaded by an admin and it would happen
only during modprobe. It's different from the commit mentioned below, on
which any user could trigger it.

> 
> As there will be a fallback allocation later, this patch is just
> to fail silently and return ret, just as commit 0ccc22f425e5
> ("sit: use __GFP_NOWARN for user controlled allocation") did.
> 
> Reported-by: Chen Wei <weichen@redhat.com>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---
>  net/sctp/probe.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/sctp/probe.c b/net/sctp/probe.c
> index 6cc2152..5bf3164 100644
> --- a/net/sctp/probe.c
> +++ b/net/sctp/probe.c
> @@ -210,7 +210,7 @@ static __init int sctpprobe_init(void)
>  
>  	init_waitqueue_head(&sctpw.wait);
>  	spin_lock_init(&sctpw.lock);
> -	if (kfifo_alloc(&sctpw.fifo, bufsize, GFP_KERNEL))
> +	if (kfifo_alloc(&sctpw.fifo, bufsize, GFP_KERNEL | __GFP_NOWARN))
>  		return ret;
>  
>  	if (!proc_create(procname, S_IRUSR, init_net.proc_net,
> -- 
> 2.1.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net] sctp: use __GFP_NOWARN for sctpw.fifo allocation
@ 2017-08-05 17:08   ` Marcelo Ricardo Leitner
  0 siblings, 0 replies; 10+ messages in thread
From: Marcelo Ricardo Leitner @ 2017-08-05 17:08 UTC (permalink / raw)
  To: Xin Long; +Cc: network dev, linux-sctp, davem, Neil Horman

On Sat, Aug 05, 2017 at 08:31:09PM +0800, Xin Long wrote:
> Chen Wei found a kernel call trace when modprobe sctp_probe with
> bufsize set with a huge value.
> 
> It's because in sctpprobe_init when alloc memory for sctpw.fifo,
> the size is got from userspace. If it is too large, kernel will
> fail and give a warning.

Yes but sctp_probe can only be loaded by an admin and it would happen
only during modprobe. It's different from the commit mentioned below, on
which any user could trigger it.

> 
> As there will be a fallback allocation later, this patch is just
> to fail silently and return ret, just as commit 0ccc22f425e5
> ("sit: use __GFP_NOWARN for user controlled allocation") did.
> 
> Reported-by: Chen Wei <weichen@redhat.com>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---
>  net/sctp/probe.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/sctp/probe.c b/net/sctp/probe.c
> index 6cc2152..5bf3164 100644
> --- a/net/sctp/probe.c
> +++ b/net/sctp/probe.c
> @@ -210,7 +210,7 @@ static __init int sctpprobe_init(void)
>  
>  	init_waitqueue_head(&sctpw.wait);
>  	spin_lock_init(&sctpw.lock);
> -	if (kfifo_alloc(&sctpw.fifo, bufsize, GFP_KERNEL))
> +	if (kfifo_alloc(&sctpw.fifo, bufsize, GFP_KERNEL | __GFP_NOWARN))
>  		return ret;
>  
>  	if (!proc_create(procname, S_IRUSR, init_net.proc_net,
> -- 
> 2.1.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net] sctp: use __GFP_NOWARN for sctpw.fifo allocation
  2017-08-05 17:08   ` Marcelo Ricardo Leitner
@ 2017-08-06  6:14     ` Xin Long
  -1 siblings, 0 replies; 10+ messages in thread
From: Xin Long @ 2017-08-06  6:14 UTC (permalink / raw)
  To: Marcelo Ricardo Leitner; +Cc: network dev, linux-sctp, davem, Neil Horman

On Sun, Aug 6, 2017 at 5:08 AM, Marcelo Ricardo Leitner
<marcelo.leitner@gmail.com> wrote:
> On Sat, Aug 05, 2017 at 08:31:09PM +0800, Xin Long wrote:
>> Chen Wei found a kernel call trace when modprobe sctp_probe with
>> bufsize set with a huge value.
>>
>> It's because in sctpprobe_init when alloc memory for sctpw.fifo,
>> the size is got from userspace. If it is too large, kernel will
>> fail and give a warning.
>
> Yes but sctp_probe can only be loaded by an admin and it would happen
> only during modprobe. It's different from the commit mentioned below, on
> which any user could trigger it.
yeah, in this way it's different, I think generally it's acceptable to have
this kinda warning call trace by admin.

But it could get the feedback from the return value and the warning
call trace seems not useful. sometimes users may be confused
with this call trace. So it may be better not to dump the warning ?

Or you think it can be helpful if we leave it here ?

>
>>
>> As there will be a fallback allocation later, this patch is just
>> to fail silently and return ret, just as commit 0ccc22f425e5
>> ("sit: use __GFP_NOWARN for user controlled allocation") did.
>>
>> Reported-by: Chen Wei <weichen@redhat.com>
>> Signed-off-by: Xin Long <lucien.xin@gmail.com>
>> ---
>>  net/sctp/probe.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/sctp/probe.c b/net/sctp/probe.c
>> index 6cc2152..5bf3164 100644
>> --- a/net/sctp/probe.c
>> +++ b/net/sctp/probe.c
>> @@ -210,7 +210,7 @@ static __init int sctpprobe_init(void)
>>
>>       init_waitqueue_head(&sctpw.wait);
>>       spin_lock_init(&sctpw.lock);
>> -     if (kfifo_alloc(&sctpw.fifo, bufsize, GFP_KERNEL))
>> +     if (kfifo_alloc(&sctpw.fifo, bufsize, GFP_KERNEL | __GFP_NOWARN))
>>               return ret;
>>
>>       if (!proc_create(procname, S_IRUSR, init_net.proc_net,
>> --
>> 2.1.0
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net] sctp: use __GFP_NOWARN for sctpw.fifo allocation
@ 2017-08-06  6:14     ` Xin Long
  0 siblings, 0 replies; 10+ messages in thread
From: Xin Long @ 2017-08-06  6:14 UTC (permalink / raw)
  To: Marcelo Ricardo Leitner; +Cc: network dev, linux-sctp, davem, Neil Horman

On Sun, Aug 6, 2017 at 5:08 AM, Marcelo Ricardo Leitner
<marcelo.leitner@gmail.com> wrote:
> On Sat, Aug 05, 2017 at 08:31:09PM +0800, Xin Long wrote:
>> Chen Wei found a kernel call trace when modprobe sctp_probe with
>> bufsize set with a huge value.
>>
>> It's because in sctpprobe_init when alloc memory for sctpw.fifo,
>> the size is got from userspace. If it is too large, kernel will
>> fail and give a warning.
>
> Yes but sctp_probe can only be loaded by an admin and it would happen
> only during modprobe. It's different from the commit mentioned below, on
> which any user could trigger it.
yeah, in this way it's different, I think generally it's acceptable to have
this kinda warning call trace by admin.

But it could get the feedback from the return value and the warning
call trace seems not useful. sometimes users may be confused
with this call trace. So it may be better not to dump the warning ?

Or you think it can be helpful if we leave it here ?

>
>>
>> As there will be a fallback allocation later, this patch is just
>> to fail silently and return ret, just as commit 0ccc22f425e5
>> ("sit: use __GFP_NOWARN for user controlled allocation") did.
>>
>> Reported-by: Chen Wei <weichen@redhat.com>
>> Signed-off-by: Xin Long <lucien.xin@gmail.com>
>> ---
>>  net/sctp/probe.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/sctp/probe.c b/net/sctp/probe.c
>> index 6cc2152..5bf3164 100644
>> --- a/net/sctp/probe.c
>> +++ b/net/sctp/probe.c
>> @@ -210,7 +210,7 @@ static __init int sctpprobe_init(void)
>>
>>       init_waitqueue_head(&sctpw.wait);
>>       spin_lock_init(&sctpw.lock);
>> -     if (kfifo_alloc(&sctpw.fifo, bufsize, GFP_KERNEL))
>> +     if (kfifo_alloc(&sctpw.fifo, bufsize, GFP_KERNEL | __GFP_NOWARN))
>>               return ret;
>>
>>       if (!proc_create(procname, S_IRUSR, init_net.proc_net,
>> --
>> 2.1.0
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net] sctp: use __GFP_NOWARN for sctpw.fifo allocation
  2017-08-06  6:14     ` Xin Long
@ 2017-08-06 23:39       ` Marcelo Ricardo Leitner
  -1 siblings, 0 replies; 10+ messages in thread
From: Marcelo Ricardo Leitner @ 2017-08-06 23:39 UTC (permalink / raw)
  To: Xin Long; +Cc: network dev, linux-sctp, davem, Neil Horman

On Sun, Aug 06, 2017 at 06:14:39PM +1200, Xin Long wrote:
> On Sun, Aug 6, 2017 at 5:08 AM, Marcelo Ricardo Leitner
> <marcelo.leitner@gmail.com> wrote:
> > On Sat, Aug 05, 2017 at 08:31:09PM +0800, Xin Long wrote:
> >> Chen Wei found a kernel call trace when modprobe sctp_probe with
> >> bufsize set with a huge value.
> >>
> >> It's because in sctpprobe_init when alloc memory for sctpw.fifo,
> >> the size is got from userspace. If it is too large, kernel will
> >> fail and give a warning.
> >
> > Yes but sctp_probe can only be loaded by an admin and it would happen
> > only during modprobe. It's different from the commit mentioned below, on
> > which any user could trigger it.
> yeah, in this way it's different, I think generally it's acceptable to have
> this kinda warning call trace by admin.
> 
> But it could get the feedback from the return value and the warning
> call trace seems not useful. sometimes users may be confused

users or admins?

> with this call trace. So it may be better not to dump the warning ?
> 
> Or you think it can be helpful if we leave it here ?

I'm afraid we may be exagerating here. There are several other ways that
an admin can trigger scary warnings, this one is no special. I'd rather
leave this one to the mm defaults instead.

> 
> >
> >>
> >> As there will be a fallback allocation later, this patch is just
> >> to fail silently and return ret, just as commit 0ccc22f425e5
> >> ("sit: use __GFP_NOWARN for user controlled allocation") did.
> >>
> >> Reported-by: Chen Wei <weichen@redhat.com>
> >> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> >> ---
> >>  net/sctp/probe.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/net/sctp/probe.c b/net/sctp/probe.c
> >> index 6cc2152..5bf3164 100644
> >> --- a/net/sctp/probe.c
> >> +++ b/net/sctp/probe.c
> >> @@ -210,7 +210,7 @@ static __init int sctpprobe_init(void)
> >>
> >>       init_waitqueue_head(&sctpw.wait);
> >>       spin_lock_init(&sctpw.lock);
> >> -     if (kfifo_alloc(&sctpw.fifo, bufsize, GFP_KERNEL))
> >> +     if (kfifo_alloc(&sctpw.fifo, bufsize, GFP_KERNEL | __GFP_NOWARN))
> >>               return ret;
> >>
> >>       if (!proc_create(procname, S_IRUSR, init_net.proc_net,
> >> --
> >> 2.1.0
> >>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> >> the body of a message to majordomo@vger.kernel.org
> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net] sctp: use __GFP_NOWARN for sctpw.fifo allocation
@ 2017-08-06 23:39       ` Marcelo Ricardo Leitner
  0 siblings, 0 replies; 10+ messages in thread
From: Marcelo Ricardo Leitner @ 2017-08-06 23:39 UTC (permalink / raw)
  To: Xin Long; +Cc: network dev, linux-sctp, davem, Neil Horman

On Sun, Aug 06, 2017 at 06:14:39PM +1200, Xin Long wrote:
> On Sun, Aug 6, 2017 at 5:08 AM, Marcelo Ricardo Leitner
> <marcelo.leitner@gmail.com> wrote:
> > On Sat, Aug 05, 2017 at 08:31:09PM +0800, Xin Long wrote:
> >> Chen Wei found a kernel call trace when modprobe sctp_probe with
> >> bufsize set with a huge value.
> >>
> >> It's because in sctpprobe_init when alloc memory for sctpw.fifo,
> >> the size is got from userspace. If it is too large, kernel will
> >> fail and give a warning.
> >
> > Yes but sctp_probe can only be loaded by an admin and it would happen
> > only during modprobe. It's different from the commit mentioned below, on
> > which any user could trigger it.
> yeah, in this way it's different, I think generally it's acceptable to have
> this kinda warning call trace by admin.
> 
> But it could get the feedback from the return value and the warning
> call trace seems not useful. sometimes users may be confused

users or admins?

> with this call trace. So it may be better not to dump the warning ?
> 
> Or you think it can be helpful if we leave it here ?

I'm afraid we may be exagerating here. There are several other ways that
an admin can trigger scary warnings, this one is no special. I'd rather
leave this one to the mm defaults instead.

> 
> >
> >>
> >> As there will be a fallback allocation later, this patch is just
> >> to fail silently and return ret, just as commit 0ccc22f425e5
> >> ("sit: use __GFP_NOWARN for user controlled allocation") did.
> >>
> >> Reported-by: Chen Wei <weichen@redhat.com>
> >> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> >> ---
> >>  net/sctp/probe.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/net/sctp/probe.c b/net/sctp/probe.c
> >> index 6cc2152..5bf3164 100644
> >> --- a/net/sctp/probe.c
> >> +++ b/net/sctp/probe.c
> >> @@ -210,7 +210,7 @@ static __init int sctpprobe_init(void)
> >>
> >>       init_waitqueue_head(&sctpw.wait);
> >>       spin_lock_init(&sctpw.lock);
> >> -     if (kfifo_alloc(&sctpw.fifo, bufsize, GFP_KERNEL))
> >> +     if (kfifo_alloc(&sctpw.fifo, bufsize, GFP_KERNEL | __GFP_NOWARN))
> >>               return ret;
> >>
> >>       if (!proc_create(procname, S_IRUSR, init_net.proc_net,
> >> --
> >> 2.1.0
> >>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> >> the body of a message to majordomo@vger.kernel.org
> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net] sctp: use __GFP_NOWARN for sctpw.fifo allocation
  2017-08-06 23:39       ` Marcelo Ricardo Leitner
@ 2017-08-08  2:35         ` Xin Long
  -1 siblings, 0 replies; 10+ messages in thread
From: Xin Long @ 2017-08-08  2:35 UTC (permalink / raw)
  To: Marcelo Ricardo Leitner; +Cc: network dev, linux-sctp, davem, Neil Horman

On Mon, Aug 7, 2017 at 11:39 AM, Marcelo Ricardo Leitner
<marcelo.leitner@gmail.com> wrote:
> On Sun, Aug 06, 2017 at 06:14:39PM +1200, Xin Long wrote:
>> On Sun, Aug 6, 2017 at 5:08 AM, Marcelo Ricardo Leitner
>> <marcelo.leitner@gmail.com> wrote:
>> > On Sat, Aug 05, 2017 at 08:31:09PM +0800, Xin Long wrote:
>> >> Chen Wei found a kernel call trace when modprobe sctp_probe with
>> >> bufsize set with a huge value.
>> >>
>> >> It's because in sctpprobe_init when alloc memory for sctpw.fifo,
>> >> the size is got from userspace. If it is too large, kernel will
>> >> fail and give a warning.
>> >
>> > Yes but sctp_probe can only be loaded by an admin and it would happen
>> > only during modprobe. It's different from the commit mentioned below, on
>> > which any user could trigger it.
>> yeah, in this way it's different, I think generally it's acceptable to have
>> this kinda warning call trace by admin.
>>
>> But it could get the feedback from the return value and the warning
>> call trace seems not useful. sometimes users may be confused
>
> users or admins?
admins.
>
>> with this call trace. So it may be better not to dump the warning ?
>>
>> Or you think it can be helpful if we leave it here ?
>
> I'm afraid we may be exagerating here. There are several other ways that
> an admin can trigger scary warnings, this one is no special. I'd rather
> leave this one to the mm defaults instead.
OK, I'm all for that.
>
>>
>> >
>> >>
>> >> As there will be a fallback allocation later, this patch is just
>> >> to fail silently and return ret, just as commit 0ccc22f425e5
>> >> ("sit: use __GFP_NOWARN for user controlled allocation") did.
>> >>
>> >> Reported-by: Chen Wei <weichen@redhat.com>
>> >> Signed-off-by: Xin Long <lucien.xin@gmail.com>
>> >> ---
>> >>  net/sctp/probe.c | 2 +-
>> >>  1 file changed, 1 insertion(+), 1 deletion(-)
>> >>
>> >> diff --git a/net/sctp/probe.c b/net/sctp/probe.c
>> >> index 6cc2152..5bf3164 100644
>> >> --- a/net/sctp/probe.c
>> >> +++ b/net/sctp/probe.c
>> >> @@ -210,7 +210,7 @@ static __init int sctpprobe_init(void)
>> >>
>> >>       init_waitqueue_head(&sctpw.wait);
>> >>       spin_lock_init(&sctpw.lock);
>> >> -     if (kfifo_alloc(&sctpw.fifo, bufsize, GFP_KERNEL))
>> >> +     if (kfifo_alloc(&sctpw.fifo, bufsize, GFP_KERNEL | __GFP_NOWARN))
>> >>               return ret;
>> >>
>> >>       if (!proc_create(procname, S_IRUSR, init_net.proc_net,
>> >> --
>> >> 2.1.0
>> >>
>> >> --
>> >> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
>> >> the body of a message to majordomo@vger.kernel.org
>> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> >>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net] sctp: use __GFP_NOWARN for sctpw.fifo allocation
@ 2017-08-08  2:35         ` Xin Long
  0 siblings, 0 replies; 10+ messages in thread
From: Xin Long @ 2017-08-08  2:35 UTC (permalink / raw)
  To: Marcelo Ricardo Leitner; +Cc: network dev, linux-sctp, davem, Neil Horman

On Mon, Aug 7, 2017 at 11:39 AM, Marcelo Ricardo Leitner
<marcelo.leitner@gmail.com> wrote:
> On Sun, Aug 06, 2017 at 06:14:39PM +1200, Xin Long wrote:
>> On Sun, Aug 6, 2017 at 5:08 AM, Marcelo Ricardo Leitner
>> <marcelo.leitner@gmail.com> wrote:
>> > On Sat, Aug 05, 2017 at 08:31:09PM +0800, Xin Long wrote:
>> >> Chen Wei found a kernel call trace when modprobe sctp_probe with
>> >> bufsize set with a huge value.
>> >>
>> >> It's because in sctpprobe_init when alloc memory for sctpw.fifo,
>> >> the size is got from userspace. If it is too large, kernel will
>> >> fail and give a warning.
>> >
>> > Yes but sctp_probe can only be loaded by an admin and it would happen
>> > only during modprobe. It's different from the commit mentioned below, on
>> > which any user could trigger it.
>> yeah, in this way it's different, I think generally it's acceptable to have
>> this kinda warning call trace by admin.
>>
>> But it could get the feedback from the return value and the warning
>> call trace seems not useful. sometimes users may be confused
>
> users or admins?
admins.
>
>> with this call trace. So it may be better not to dump the warning ?
>>
>> Or you think it can be helpful if we leave it here ?
>
> I'm afraid we may be exagerating here. There are several other ways that
> an admin can trigger scary warnings, this one is no special. I'd rather
> leave this one to the mm defaults instead.
OK, I'm all for that.
>
>>
>> >
>> >>
>> >> As there will be a fallback allocation later, this patch is just
>> >> to fail silently and return ret, just as commit 0ccc22f425e5
>> >> ("sit: use __GFP_NOWARN for user controlled allocation") did.
>> >>
>> >> Reported-by: Chen Wei <weichen@redhat.com>
>> >> Signed-off-by: Xin Long <lucien.xin@gmail.com>
>> >> ---
>> >>  net/sctp/probe.c | 2 +-
>> >>  1 file changed, 1 insertion(+), 1 deletion(-)
>> >>
>> >> diff --git a/net/sctp/probe.c b/net/sctp/probe.c
>> >> index 6cc2152..5bf3164 100644
>> >> --- a/net/sctp/probe.c
>> >> +++ b/net/sctp/probe.c
>> >> @@ -210,7 +210,7 @@ static __init int sctpprobe_init(void)
>> >>
>> >>       init_waitqueue_head(&sctpw.wait);
>> >>       spin_lock_init(&sctpw.lock);
>> >> -     if (kfifo_alloc(&sctpw.fifo, bufsize, GFP_KERNEL))
>> >> +     if (kfifo_alloc(&sctpw.fifo, bufsize, GFP_KERNEL | __GFP_NOWARN))
>> >>               return ret;
>> >>
>> >>       if (!proc_create(procname, S_IRUSR, init_net.proc_net,
>> >> --
>> >> 2.1.0
>> >>
>> >> --
>> >> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
>> >> the body of a message to majordomo@vger.kernel.org
>> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> >>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2017-08-08  2:35 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-05 12:31 [PATCH net] sctp: use __GFP_NOWARN for sctpw.fifo allocation Xin Long
2017-08-05 12:31 ` Xin Long
2017-08-05 17:08 ` Marcelo Ricardo Leitner
2017-08-05 17:08   ` Marcelo Ricardo Leitner
2017-08-06  6:14   ` Xin Long
2017-08-06  6:14     ` Xin Long
2017-08-06 23:39     ` Marcelo Ricardo Leitner
2017-08-06 23:39       ` Marcelo Ricardo Leitner
2017-08-08  2:35       ` Xin Long
2017-08-08  2:35         ` Xin Long

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.