Re: [PATCH] proc/sysctl: fix return error for proc_doulongvec_minmax

2018-12-06 Thread Kees Cook
On Thu, Dec 6, 2018 at 12:52 AM Luis Chamberlain  wrote:
>
> On Thu, Dec 06, 2018 at 03:36:15PM +0800, Cheng Lin wrote:
> > If the number of input parameters is less than the total
> > parameters, an EINVAL error will be returned.
> >
> > e.g.
> > We use proc_doulongvec_minmax to pass up to two parameters
> > with kern_table.
> >
> > {
> >   .procname   = "monitor_signals",
> >   .data   = _sigs,
> >   .maxlen = 2*sizeof(unsigned long),
> >   .mode   = 0644,
> >   .proc_handler   = proc_doulongvec_minmax,
> > },
> >
> > Reproduce:
> > When passing two parameters, it's work normal. But passing
> > only one parameter, an error "Invalid argument"(EINVAL) is
> > returned.
> >
> > [root@cl150 ~]# echo 1 2 > /proc/sys/kernel/monitor_signals
> > [root@cl150 ~]# cat /proc/sys/kernel/monitor_signals
> > 1   2
> > [root@cl150 ~]# echo 3 > /proc/sys/kernel/monitor_signals
> > -bash: echo: write error: Invalid argument
> > [root@cl150 ~]# echo $?
> > 1
> > [root@cl150 ~]# cat /proc/sys/kernel/monitor_signals
> > 3   2
> > [root@cl150 ~]#
> >
> > The following is the result after apply this patch. No error
> > is returned when the number of input parameters is less than
> > the total parameters.
> >
> > [root@cl150 ~]# echo 1 2 > /proc/sys/kernel/monitor_signals
> > [root@cl150 ~]# cat /proc/sys/kernel/monitor_signals
> > 1   2
> > [root@cl150 ~]# echo 3 > /proc/sys/kernel/monitor_signals
> > [root@cl150 ~]# echo $?
> > 0
> > [root@cl150 ~]# cat /proc/sys/kernel/monitor_signals
> > 3   2
> > [root@cl150 ~]#
> >
> > There are three processing functions dealing with digital parameters,
> > __do_proc_dointvec/__do_proc_douintvec/__do_proc_doulongvec_minmax.
> >
> > This patch deals with __do_proc_doulongvec_minmax, just as
> > __do_proc_dointvec does, adding a check for parameters 'left'. In
> > __do_proc_douintvec, its code implementation explicitly does not
> > support multiple inputs.
> >
> > static int __do_proc_douintvec(...){
> >  ...
> >  /*
> >   * Arrays are not supported, keep this simple. *Do not* add
> >   * support for them.
> >   */
> >  if (vleft != 1) {
> >  *lenp = 0;
> >  return -EINVAL;
> >  }
> >  ...
> > }
> >
> > So, just __do_proc_doulongvec_minmax has the problem. And most use of
> > proc_doulongvec_minmax/proc_doulongvec_ms_jiffies_minmax just have one
> > parameter.
> >
> > Signed-off-by: Cheng Lin 
>
> Thanks for fixing up the commit log.
>
> Acked-by: Luis Chamberlain 

Reviewed-by: Kees Cook 

-Kees

>
> I think we can live with this outside of stable. So stable is not
> needed. But I would not be surprised if autosel algorithm will end
> up picking it up. And if so.. well, it cannot hurt.
>
>   Luis



-- 
Kees Cook


Re: [PATCH] proc/sysctl: fix return error for proc_doulongvec_minmax

2018-12-06 Thread Kees Cook
On Thu, Dec 6, 2018 at 12:52 AM Luis Chamberlain  wrote:
>
> On Thu, Dec 06, 2018 at 03:36:15PM +0800, Cheng Lin wrote:
> > If the number of input parameters is less than the total
> > parameters, an EINVAL error will be returned.
> >
> > e.g.
> > We use proc_doulongvec_minmax to pass up to two parameters
> > with kern_table.
> >
> > {
> >   .procname   = "monitor_signals",
> >   .data   = _sigs,
> >   .maxlen = 2*sizeof(unsigned long),
> >   .mode   = 0644,
> >   .proc_handler   = proc_doulongvec_minmax,
> > },
> >
> > Reproduce:
> > When passing two parameters, it's work normal. But passing
> > only one parameter, an error "Invalid argument"(EINVAL) is
> > returned.
> >
> > [root@cl150 ~]# echo 1 2 > /proc/sys/kernel/monitor_signals
> > [root@cl150 ~]# cat /proc/sys/kernel/monitor_signals
> > 1   2
> > [root@cl150 ~]# echo 3 > /proc/sys/kernel/monitor_signals
> > -bash: echo: write error: Invalid argument
> > [root@cl150 ~]# echo $?
> > 1
> > [root@cl150 ~]# cat /proc/sys/kernel/monitor_signals
> > 3   2
> > [root@cl150 ~]#
> >
> > The following is the result after apply this patch. No error
> > is returned when the number of input parameters is less than
> > the total parameters.
> >
> > [root@cl150 ~]# echo 1 2 > /proc/sys/kernel/monitor_signals
> > [root@cl150 ~]# cat /proc/sys/kernel/monitor_signals
> > 1   2
> > [root@cl150 ~]# echo 3 > /proc/sys/kernel/monitor_signals
> > [root@cl150 ~]# echo $?
> > 0
> > [root@cl150 ~]# cat /proc/sys/kernel/monitor_signals
> > 3   2
> > [root@cl150 ~]#
> >
> > There are three processing functions dealing with digital parameters,
> > __do_proc_dointvec/__do_proc_douintvec/__do_proc_doulongvec_minmax.
> >
> > This patch deals with __do_proc_doulongvec_minmax, just as
> > __do_proc_dointvec does, adding a check for parameters 'left'. In
> > __do_proc_douintvec, its code implementation explicitly does not
> > support multiple inputs.
> >
> > static int __do_proc_douintvec(...){
> >  ...
> >  /*
> >   * Arrays are not supported, keep this simple. *Do not* add
> >   * support for them.
> >   */
> >  if (vleft != 1) {
> >  *lenp = 0;
> >  return -EINVAL;
> >  }
> >  ...
> > }
> >
> > So, just __do_proc_doulongvec_minmax has the problem. And most use of
> > proc_doulongvec_minmax/proc_doulongvec_ms_jiffies_minmax just have one
> > parameter.
> >
> > Signed-off-by: Cheng Lin 
>
> Thanks for fixing up the commit log.
>
> Acked-by: Luis Chamberlain 

Reviewed-by: Kees Cook 

-Kees

>
> I think we can live with this outside of stable. So stable is not
> needed. But I would not be surprised if autosel algorithm will end
> up picking it up. And if so.. well, it cannot hurt.
>
>   Luis



-- 
Kees Cook


Re: [PATCH] proc/sysctl: fix return error for proc_doulongvec_minmax

2018-12-06 Thread Luis Chamberlain
On Thu, Dec 06, 2018 at 03:36:15PM +0800, Cheng Lin wrote:
> If the number of input parameters is less than the total
> parameters, an EINVAL error will be returned.
> 
> e.g.
> We use proc_doulongvec_minmax to pass up to two parameters
> with kern_table.
> 
> {
>   .procname   = "monitor_signals",
>   .data   = _sigs,
>   .maxlen = 2*sizeof(unsigned long),
>   .mode   = 0644,
>   .proc_handler   = proc_doulongvec_minmax,
> },
> 
> Reproduce:
> When passing two parameters, it's work normal. But passing
> only one parameter, an error "Invalid argument"(EINVAL) is
> returned.
> 
> [root@cl150 ~]# echo 1 2 > /proc/sys/kernel/monitor_signals
> [root@cl150 ~]# cat /proc/sys/kernel/monitor_signals
> 1   2
> [root@cl150 ~]# echo 3 > /proc/sys/kernel/monitor_signals
> -bash: echo: write error: Invalid argument
> [root@cl150 ~]# echo $?
> 1
> [root@cl150 ~]# cat /proc/sys/kernel/monitor_signals
> 3   2
> [root@cl150 ~]#
> 
> The following is the result after apply this patch. No error
> is returned when the number of input parameters is less than
> the total parameters.
> 
> [root@cl150 ~]# echo 1 2 > /proc/sys/kernel/monitor_signals
> [root@cl150 ~]# cat /proc/sys/kernel/monitor_signals
> 1   2
> [root@cl150 ~]# echo 3 > /proc/sys/kernel/monitor_signals
> [root@cl150 ~]# echo $?
> 0
> [root@cl150 ~]# cat /proc/sys/kernel/monitor_signals
> 3   2
> [root@cl150 ~]#
> 
> There are three processing functions dealing with digital parameters,
> __do_proc_dointvec/__do_proc_douintvec/__do_proc_doulongvec_minmax.
> 
> This patch deals with __do_proc_doulongvec_minmax, just as
> __do_proc_dointvec does, adding a check for parameters 'left'. In
> __do_proc_douintvec, its code implementation explicitly does not
> support multiple inputs.
> 
> static int __do_proc_douintvec(...){
>  ...
>  /*
>   * Arrays are not supported, keep this simple. *Do not* add
>   * support for them.
>   */
>  if (vleft != 1) {
>  *lenp = 0;
>  return -EINVAL;
>  }
>  ...
> }
> 
> So, just __do_proc_doulongvec_minmax has the problem. And most use of
> proc_doulongvec_minmax/proc_doulongvec_ms_jiffies_minmax just have one
> parameter.
> 
> Signed-off-by: Cheng Lin 

Thanks for fixing up the commit log.

Acked-by: Luis Chamberlain 

I think we can live with this outside of stable. So stable is not
needed. But I would not be surprised if autosel algorithm will end
up picking it up. And if so.. well, it cannot hurt.

  Luis


Re: [PATCH] proc/sysctl: fix return error for proc_doulongvec_minmax

2018-12-06 Thread Luis Chamberlain
On Thu, Dec 06, 2018 at 03:36:15PM +0800, Cheng Lin wrote:
> If the number of input parameters is less than the total
> parameters, an EINVAL error will be returned.
> 
> e.g.
> We use proc_doulongvec_minmax to pass up to two parameters
> with kern_table.
> 
> {
>   .procname   = "monitor_signals",
>   .data   = _sigs,
>   .maxlen = 2*sizeof(unsigned long),
>   .mode   = 0644,
>   .proc_handler   = proc_doulongvec_minmax,
> },
> 
> Reproduce:
> When passing two parameters, it's work normal. But passing
> only one parameter, an error "Invalid argument"(EINVAL) is
> returned.
> 
> [root@cl150 ~]# echo 1 2 > /proc/sys/kernel/monitor_signals
> [root@cl150 ~]# cat /proc/sys/kernel/monitor_signals
> 1   2
> [root@cl150 ~]# echo 3 > /proc/sys/kernel/monitor_signals
> -bash: echo: write error: Invalid argument
> [root@cl150 ~]# echo $?
> 1
> [root@cl150 ~]# cat /proc/sys/kernel/monitor_signals
> 3   2
> [root@cl150 ~]#
> 
> The following is the result after apply this patch. No error
> is returned when the number of input parameters is less than
> the total parameters.
> 
> [root@cl150 ~]# echo 1 2 > /proc/sys/kernel/monitor_signals
> [root@cl150 ~]# cat /proc/sys/kernel/monitor_signals
> 1   2
> [root@cl150 ~]# echo 3 > /proc/sys/kernel/monitor_signals
> [root@cl150 ~]# echo $?
> 0
> [root@cl150 ~]# cat /proc/sys/kernel/monitor_signals
> 3   2
> [root@cl150 ~]#
> 
> There are three processing functions dealing with digital parameters,
> __do_proc_dointvec/__do_proc_douintvec/__do_proc_doulongvec_minmax.
> 
> This patch deals with __do_proc_doulongvec_minmax, just as
> __do_proc_dointvec does, adding a check for parameters 'left'. In
> __do_proc_douintvec, its code implementation explicitly does not
> support multiple inputs.
> 
> static int __do_proc_douintvec(...){
>  ...
>  /*
>   * Arrays are not supported, keep this simple. *Do not* add
>   * support for them.
>   */
>  if (vleft != 1) {
>  *lenp = 0;
>  return -EINVAL;
>  }
>  ...
> }
> 
> So, just __do_proc_doulongvec_minmax has the problem. And most use of
> proc_doulongvec_minmax/proc_doulongvec_ms_jiffies_minmax just have one
> parameter.
> 
> Signed-off-by: Cheng Lin 

Thanks for fixing up the commit log.

Acked-by: Luis Chamberlain 

I think we can live with this outside of stable. So stable is not
needed. But I would not be surprised if autosel algorithm will end
up picking it up. And if so.. well, it cannot hurt.

  Luis


[PATCH] proc/sysctl: fix return error for proc_doulongvec_minmax

2018-12-05 Thread Cheng Lin
If the number of input parameters is less than the total
parameters, an EINVAL error will be returned.

e.g.
We use proc_doulongvec_minmax to pass up to two parameters
with kern_table.

{
.procname   = "monitor_signals",
.data   = _sigs,
.maxlen = 2*sizeof(unsigned long),
.mode   = 0644,
.proc_handler   = proc_doulongvec_minmax,
},

Reproduce:
When passing two parameters, it's work normal. But passing
only one parameter, an error "Invalid argument"(EINVAL) is
returned.

[root@cl150 ~]# echo 1 2 > /proc/sys/kernel/monitor_signals
[root@cl150 ~]# cat /proc/sys/kernel/monitor_signals
1   2
[root@cl150 ~]# echo 3 > /proc/sys/kernel/monitor_signals
-bash: echo: write error: Invalid argument
[root@cl150 ~]# echo $?
1
[root@cl150 ~]# cat /proc/sys/kernel/monitor_signals
3   2
[root@cl150 ~]#

The following is the result after apply this patch. No error
is returned when the number of input parameters is less than
the total parameters.

[root@cl150 ~]# echo 1 2 > /proc/sys/kernel/monitor_signals
[root@cl150 ~]# cat /proc/sys/kernel/monitor_signals
1   2
[root@cl150 ~]# echo 3 > /proc/sys/kernel/monitor_signals
[root@cl150 ~]# echo $?
0
[root@cl150 ~]# cat /proc/sys/kernel/monitor_signals
3   2
[root@cl150 ~]#

There are three processing functions dealing with digital parameters,
__do_proc_dointvec/__do_proc_douintvec/__do_proc_doulongvec_minmax.

This patch deals with __do_proc_doulongvec_minmax, just as
__do_proc_dointvec does, adding a check for parameters 'left'. In
__do_proc_douintvec, its code implementation explicitly does not
support multiple inputs.

static int __do_proc_douintvec(...){
 ...
 /*
  * Arrays are not supported, keep this simple. *Do not* add
  * support for them.
  */
 if (vleft != 1) {
 *lenp = 0;
 return -EINVAL;
 }
 ...
}

So, just __do_proc_doulongvec_minmax has the problem. And most use of
proc_doulongvec_minmax/proc_doulongvec_ms_jiffies_minmax just have one
parameter.

Signed-off-by: Cheng Lin 
---
 kernel/sysctl.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 5fc724e..9ee261f 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -2779,6 +2779,8 @@ static int __do_proc_doulongvec_minmax(void *data, struct 
ctl_table *table, int
bool neg;
 
left -= proc_skip_spaces();
+   if (!left)
+   break;
 
err = proc_get_long(, , , ,
 proc_wspace_sep,
-- 
1.8.3.1



[PATCH] proc/sysctl: fix return error for proc_doulongvec_minmax

2018-12-05 Thread Cheng Lin
If the number of input parameters is less than the total
parameters, an EINVAL error will be returned.

e.g.
We use proc_doulongvec_minmax to pass up to two parameters
with kern_table.

{
.procname   = "monitor_signals",
.data   = _sigs,
.maxlen = 2*sizeof(unsigned long),
.mode   = 0644,
.proc_handler   = proc_doulongvec_minmax,
},

Reproduce:
When passing two parameters, it's work normal. But passing
only one parameter, an error "Invalid argument"(EINVAL) is
returned.

[root@cl150 ~]# echo 1 2 > /proc/sys/kernel/monitor_signals
[root@cl150 ~]# cat /proc/sys/kernel/monitor_signals
1   2
[root@cl150 ~]# echo 3 > /proc/sys/kernel/monitor_signals
-bash: echo: write error: Invalid argument
[root@cl150 ~]# echo $?
1
[root@cl150 ~]# cat /proc/sys/kernel/monitor_signals
3   2
[root@cl150 ~]#

The following is the result after apply this patch. No error
is returned when the number of input parameters is less than
the total parameters.

[root@cl150 ~]# echo 1 2 > /proc/sys/kernel/monitor_signals
[root@cl150 ~]# cat /proc/sys/kernel/monitor_signals
1   2
[root@cl150 ~]# echo 3 > /proc/sys/kernel/monitor_signals
[root@cl150 ~]# echo $?
0
[root@cl150 ~]# cat /proc/sys/kernel/monitor_signals
3   2
[root@cl150 ~]#

There are three processing functions dealing with digital parameters,
__do_proc_dointvec/__do_proc_douintvec/__do_proc_doulongvec_minmax.

This patch deals with __do_proc_doulongvec_minmax, just as
__do_proc_dointvec does, adding a check for parameters 'left'. In
__do_proc_douintvec, its code implementation explicitly does not
support multiple inputs.

static int __do_proc_douintvec(...){
 ...
 /*
  * Arrays are not supported, keep this simple. *Do not* add
  * support for them.
  */
 if (vleft != 1) {
 *lenp = 0;
 return -EINVAL;
 }
 ...
}

So, just __do_proc_doulongvec_minmax has the problem. And most use of
proc_doulongvec_minmax/proc_doulongvec_ms_jiffies_minmax just have one
parameter.

Signed-off-by: Cheng Lin 
---
 kernel/sysctl.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 5fc724e..9ee261f 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -2779,6 +2779,8 @@ static int __do_proc_doulongvec_minmax(void *data, struct 
ctl_table *table, int
bool neg;
 
left -= proc_skip_spaces();
+   if (!left)
+   break;
 
err = proc_get_long(, , , ,
 proc_wspace_sep,
-- 
1.8.3.1



Re: Re: [PATCH] proc/sysctl: fix return error for proc_doulongvec_minmax

2018-12-05 Thread Kees Cook
On Mon, Dec 3, 2018 at 12:14 PM Luis Chamberlain  wrote:
> Since this worked before I do agree that we need to keep it working now,
> and I can't think of an issue with returning 0 now. Since this is about
> semantics though I'd like a bit more review from at last one more
> person.
>
> Kees, Eric, Andrew?

This is a weird one: it would return an error _AND_ still perform the
write. :( I think this patch is right, and I struggle to imagine a
case where removing the failure is a problem.

A quick question, though, do we want to instead do the reverse? (Not
update, and keep the error?) Are there any examples of doing partial
writes like this in real software?

The proposed change is the safest change, though...

-- 
Kees Cook


Re: Re: [PATCH] proc/sysctl: fix return error for proc_doulongvec_minmax

2018-12-05 Thread Kees Cook
On Mon, Dec 3, 2018 at 12:14 PM Luis Chamberlain  wrote:
> Since this worked before I do agree that we need to keep it working now,
> and I can't think of an issue with returning 0 now. Since this is about
> semantics though I'd like a bit more review from at last one more
> person.
>
> Kees, Eric, Andrew?

This is a weird one: it would return an error _AND_ still perform the
write. :( I think this patch is right, and I struggle to imagine a
case where removing the failure is a problem.

A quick question, though, do we want to instead do the reverse? (Not
update, and keep the error?) Are there any examples of doing partial
writes like this in real software?

The proposed change is the safest change, though...

-- 
Kees Cook


Re: Re: [PATCH] proc/sysctl: fix return error for proc_doulongvec_minmax

2018-12-03 Thread Luis Chamberlain
On Mon, Dec 03, 2018 at 01:12:39PM +0800, cheng.lin...@zte.com.cn wrote:
> >Cheng, thanks for the patch!
> >
> >On Fri, Nov 30, 2018 at 02:35:17PM +0800, Cheng Lin wrote:
> >> If the number of input parameters is less than the total
> >> parameters, an INVAL error will be returned.
> >
> >Do you mean EINVAL?
> >
> Yes, it's EINVAL.

Please adjust the commit log.

> >> This patch ensure no error returned in this condition, just
> >> like other interfaces do.
> >
> >Have an actual example to reproduce?
> >
> >Luis
> >
> We use proc_doulongvec_minmax to pass up to two parameters with kern_table.
> e.g. 
> {
> .procname   = "monitor_signals",
> .data   = _sigs,
> .maxlen = 2*sizeof(unsigned long),
> .mode   = 0644,
> .proc_handler   = proc_doulongvec_minmax,
> },
> 
> Reproduce:
> When passing two parameters, it's work normal. But passing only one 
> parameter, an error "Invalid argument"(EINVAL) is returned.
> [root@cl150 ~]# echo 1 2 > /proc/sys/kernel/monitor_signals
> [root@cl150 ~]# cat /proc/sys/kernel/monitor_signals
> 1   2
> [root@cl150 ~]# echo 3 > /proc/sys/kernel/monitor_signals
> -bash: echo: write error: Invalid argument
> [root@cl150 ~]# echo $?
> 1
> [root@cl150 ~]# cat /proc/sys/kernel/monitor_signals
> 3   2
> [root@cl150 ~]#
> 
> The following is the result after apply this patch. No error is returned when 
> the number of input parameters is less than the total parameters.
> [root@cl150 ~]# echo 1 2 > /proc/sys/kernel/monitor_signals
> [root@cl150 ~]# cat /proc/sys/kernel/monitor_signals
> 1   2
> [root@cl150 ~]# echo 3 > /proc/sys/kernel/monitor_signals
> [root@cl150 ~]# echo $?
> 0
> [root@cl150 ~]# cat /proc/sys/kernel/monitor_signals
> 3   2
> [root@cl150 ~]#

This would be good to have in the commit log as well. But your patch
only addresses one of the proc users, there are a few other checks like
this that would also need to be expanded for this. So please expand
your patch to cover the other cases as well.

Since this worked before I do agree that we need to keep it working now,
and I can't think of an issue with returning 0 now. Since this is about
semantics though I'd like a bit more review from at last one more
person.

Kees, Eric, Andrew?

  Luis

> Cheng
> 
> >> Signed-off-by: Cheng Lin 
> >> ---
> >>  kernel/sysctl.c | 2 ++
> >>  1 file changed, 2 insertions(+)
> >>
> >> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> >> index 5fc724e..9ee261f 100644
> >> --- a/kernel/sysctl.c
> >> +++ b/kernel/sysctl.c
> >> @@ -2779,6 +2779,8 @@ static int __do_proc_doulongvec_minmax(void *data, 
> >> struct ctl_table *table, int
> >>  bool neg;
> >>
> >>  left -= proc_skip_spaces();
> >> +if (!left)
> >> +break;
> >>
> >>  err = proc_get_long(, , , ,
> >>   proc_wspace_sep,
> >> --
> >> 1.8.3.1
> >>



Re: Re: [PATCH] proc/sysctl: fix return error for proc_doulongvec_minmax

2018-12-03 Thread Luis Chamberlain
On Mon, Dec 03, 2018 at 01:12:39PM +0800, cheng.lin...@zte.com.cn wrote:
> >Cheng, thanks for the patch!
> >
> >On Fri, Nov 30, 2018 at 02:35:17PM +0800, Cheng Lin wrote:
> >> If the number of input parameters is less than the total
> >> parameters, an INVAL error will be returned.
> >
> >Do you mean EINVAL?
> >
> Yes, it's EINVAL.

Please adjust the commit log.

> >> This patch ensure no error returned in this condition, just
> >> like other interfaces do.
> >
> >Have an actual example to reproduce?
> >
> >Luis
> >
> We use proc_doulongvec_minmax to pass up to two parameters with kern_table.
> e.g. 
> {
> .procname   = "monitor_signals",
> .data   = _sigs,
> .maxlen = 2*sizeof(unsigned long),
> .mode   = 0644,
> .proc_handler   = proc_doulongvec_minmax,
> },
> 
> Reproduce:
> When passing two parameters, it's work normal. But passing only one 
> parameter, an error "Invalid argument"(EINVAL) is returned.
> [root@cl150 ~]# echo 1 2 > /proc/sys/kernel/monitor_signals
> [root@cl150 ~]# cat /proc/sys/kernel/monitor_signals
> 1   2
> [root@cl150 ~]# echo 3 > /proc/sys/kernel/monitor_signals
> -bash: echo: write error: Invalid argument
> [root@cl150 ~]# echo $?
> 1
> [root@cl150 ~]# cat /proc/sys/kernel/monitor_signals
> 3   2
> [root@cl150 ~]#
> 
> The following is the result after apply this patch. No error is returned when 
> the number of input parameters is less than the total parameters.
> [root@cl150 ~]# echo 1 2 > /proc/sys/kernel/monitor_signals
> [root@cl150 ~]# cat /proc/sys/kernel/monitor_signals
> 1   2
> [root@cl150 ~]# echo 3 > /proc/sys/kernel/monitor_signals
> [root@cl150 ~]# echo $?
> 0
> [root@cl150 ~]# cat /proc/sys/kernel/monitor_signals
> 3   2
> [root@cl150 ~]#

This would be good to have in the commit log as well. But your patch
only addresses one of the proc users, there are a few other checks like
this that would also need to be expanded for this. So please expand
your patch to cover the other cases as well.

Since this worked before I do agree that we need to keep it working now,
and I can't think of an issue with returning 0 now. Since this is about
semantics though I'd like a bit more review from at last one more
person.

Kees, Eric, Andrew?

  Luis

> Cheng
> 
> >> Signed-off-by: Cheng Lin 
> >> ---
> >>  kernel/sysctl.c | 2 ++
> >>  1 file changed, 2 insertions(+)
> >>
> >> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> >> index 5fc724e..9ee261f 100644
> >> --- a/kernel/sysctl.c
> >> +++ b/kernel/sysctl.c
> >> @@ -2779,6 +2779,8 @@ static int __do_proc_doulongvec_minmax(void *data, 
> >> struct ctl_table *table, int
> >>  bool neg;
> >>
> >>  left -= proc_skip_spaces();
> >> +if (!left)
> >> +break;
> >>
> >>  err = proc_get_long(, , , ,
> >>   proc_wspace_sep,
> >> --
> >> 1.8.3.1
> >>



Re: [PATCH] proc/sysctl: fix return error for proc_doulongvec_minmax

2018-11-30 Thread Luis Chamberlain
Cheng, thanks for the patch!

On Fri, Nov 30, 2018 at 02:35:17PM +0800, Cheng Lin wrote:
> If the number of input parameters is less than the total
> parameters, an INVAL error will be returned.

Do you mean EINVAL?

> This patch ensure no error returned in this condition, just
> like other interfaces do.

Have an actual example to reproduce?

  Luis

> Signed-off-by: Cheng Lin 
> ---
>  kernel/sysctl.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index 5fc724e..9ee261f 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -2779,6 +2779,8 @@ static int __do_proc_doulongvec_minmax(void *data, 
> struct ctl_table *table, int
>   bool neg;
>  
>   left -= proc_skip_spaces();
> + if (!left)
> + break;
>  
>   err = proc_get_long(, , , ,
>proc_wspace_sep,
> -- 
> 1.8.3.1
> 


Re: [PATCH] proc/sysctl: fix return error for proc_doulongvec_minmax

2018-11-30 Thread Luis Chamberlain
Cheng, thanks for the patch!

On Fri, Nov 30, 2018 at 02:35:17PM +0800, Cheng Lin wrote:
> If the number of input parameters is less than the total
> parameters, an INVAL error will be returned.

Do you mean EINVAL?

> This patch ensure no error returned in this condition, just
> like other interfaces do.

Have an actual example to reproduce?

  Luis

> Signed-off-by: Cheng Lin 
> ---
>  kernel/sysctl.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index 5fc724e..9ee261f 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -2779,6 +2779,8 @@ static int __do_proc_doulongvec_minmax(void *data, 
> struct ctl_table *table, int
>   bool neg;
>  
>   left -= proc_skip_spaces();
> + if (!left)
> + break;
>  
>   err = proc_get_long(, , , ,
>proc_wspace_sep,
> -- 
> 1.8.3.1
> 


[PATCH] proc/sysctl: fix return error for proc_doulongvec_minmax

2018-11-29 Thread Cheng Lin
If the number of input parameters is less than the total
parameters, an INVAL error will be returned.

This patch ensure no error returned in this condition, just
like other interfaces do.

Signed-off-by: Cheng Lin 
---
 kernel/sysctl.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 5fc724e..9ee261f 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -2779,6 +2779,8 @@ static int __do_proc_doulongvec_minmax(void *data, struct 
ctl_table *table, int
bool neg;
 
left -= proc_skip_spaces();
+   if (!left)
+   break;
 
err = proc_get_long(, , , ,
 proc_wspace_sep,
-- 
1.8.3.1



[PATCH] proc/sysctl: fix return error for proc_doulongvec_minmax

2018-11-29 Thread Cheng Lin
If the number of input parameters is less than the total
parameters, an INVAL error will be returned.

This patch ensure no error returned in this condition, just
like other interfaces do.

Signed-off-by: Cheng Lin 
---
 kernel/sysctl.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 5fc724e..9ee261f 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -2779,6 +2779,8 @@ static int __do_proc_doulongvec_minmax(void *data, struct 
ctl_table *table, int
bool neg;
 
left -= proc_skip_spaces();
+   if (!left)
+   break;
 
err = proc_get_long(, , , ,
 proc_wspace_sep,
-- 
1.8.3.1