Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1530640 > unrolled thread

[RFC] kernel/sysctl.c: return -EINVAL when write invalid val to ulong type sysctl

Started byYisheng Xie <xieyisheng1@huawei.com>
First post2016-11-26 10:20 +0100
Last post2016-11-29 11:30 +0100
Articles 3 — 2 participants

Back to article view | Back to linux.kernel


Contents

  [RFC] kernel/sysctl.c: return -EINVAL when write invalid val to ulong  type sysctl Yisheng Xie <xieyisheng1@huawei.com> - 2016-11-26 10:20 +0100
    Re: [RFC] kernel/sysctl.c: return -EINVAL when write invalid val to  ulong type sysctl subashab@codeaurora.org - 2016-11-27 00:20 +0100
      Re: [RFC] kernel/sysctl.c: return -EINVAL when write invalid val to  ulong type sysctl Yisheng Xie <xieyisheng1@huawei.com> - 2016-11-29 11:30 +0100

#1530640 — [RFC] kernel/sysctl.c: return -EINVAL when write invalid val to ulong type sysctl

FromYisheng Xie <xieyisheng1@huawei.com>
Date2016-11-26 10:20 +0100
Subject[RFC] kernel/sysctl.c: return -EINVAL when write invalid val to ulong type sysctl
Message-ID<sHHjk-4Am-13@gated-at.bofh.it>
I tried to echo an invalid value to an unsigned long type sysctl on 4.9.0-rc6:
   linux:~# cat /proc/sys/vm/user_reserve_kbytes
   131072
   linux:~# echo -1 > /proc/sys/vm/user_reserve_kbytes
   linux:~# cat /proc/sys/vm/user_reserve_kbytes
   131072

The echo operation got error and the value do not write to user_reserve_kbytes,
however, user do not know it until check the value again.

Is it more suitable to return -EINVAL when echo an invalid value to an unsigned long
type sysctl, in order to let user know what happened without checking its value once more?
Just as what int type sysctl do:
   linux:~#cat /proc/sys/kernel/sysctl_writes_strict
   1
   linux:~# echo 3 > /proc/sys/kernel/sysctl_writes_strict
   bash: echo: write error: Invalid argument

----------------------
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 706309f..40e9285 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -2485,10 +2485,14 @@ static int __do_proc_doulongvec_minmax(void *data, struct ctl_table *table, int
                                             sizeof(proc_wspace_sep), NULL);
                        if (err)
                                break;
-                       if (neg)
-                               continue;
-                       if ((min && val < *min) || (max && val > *max))
-                               continue;
+                       if (neg) {
+                               err = -EINVAL;
+                               break;
+                       }
+                       if ((min && val < *min) || (max && val > *max)) {
+                               err = -EINVAL;
+                               break;
+                       }
                        *i = val;
                } else {
                        val = convdiv * (*i) / convmul;

[toc] | [next] | [standalone]


#1530799 — Re: [RFC] kernel/sysctl.c: return -EINVAL when write invalid val to ulong type sysctl

Fromsubashab@codeaurora.org
Date2016-11-27 00:20 +0100
SubjectRe: [RFC] kernel/sysctl.c: return -EINVAL when write invalid val to ulong type sysctl
Message-ID<sHUqe-4EY-15@gated-at.bofh.it>
In reply to#1530640
On 2016-11-26 02:13, Yisheng Xie wrote:
> I tried to echo an invalid value to an unsigned long type sysctl on 
> 4.9.0-rc6:
>    linux:~# cat /proc/sys/vm/user_reserve_kbytes
>    131072
>    linux:~# echo -1 > /proc/sys/vm/user_reserve_kbytes
>    linux:~# cat /proc/sys/vm/user_reserve_kbytes
>    131072
> 
> The echo operation got error and the value do not write to 
> user_reserve_kbytes,
> however, user do not know it until check the value again.
> 
> Is it more suitable to return -EINVAL when echo an invalid value to an
> unsigned long
> type sysctl, in order to let user know what happened without checking
> its value once more?
> Just as what int type sysctl do:
>    linux:~#cat /proc/sys/kernel/sysctl_writes_strict
>    1
>    linux:~# echo 3 > /proc/sys/kernel/sysctl_writes_strict
>    bash: echo: write error: Invalid argument
> 
> ----------------------
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index 706309f..40e9285 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -2485,10 +2485,14 @@ static int __do_proc_doulongvec_minmax(void
> *data, struct ctl_table *table, int
>                                              sizeof(proc_wspace_sep), 
> NULL);
>                         if (err)
>                                 break;
> -                       if (neg)
> -                               continue;
> -                       if ((min && val < *min) || (max && val > *max))
> -                               continue;
> +                       if (neg) {
> +                               err = -EINVAL;
> +                               break;
> +                       }
> +                       if ((min && val < *min) || (max && val > *max)) 
> {
> +                               err = -EINVAL;
> +                               break;
> +                       }
>                         *i = val;
>                 } else {
>                         val = convdiv * (*i) / convmul;

Agree, this should be similar to proc_douintvec

root@vm:~# echo 8192 > /proc/sys/net/core/xfrm_aevent_rseqth
root@vm:~# cat /proc/sys/net/core/xfrm_aevent_rseqth
8192
root@vm:~# echo -1 > /proc/sys/net/core/xfrm_aevent_rseqth
-bash: echo: write error: Invalid argument
root@vm:~# cat /proc/sys/net/core/xfrm_aevent_rseqth
8192

--
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a 
Linux Foundation Collaborative Project

[toc] | [prev] | [next] | [standalone]


#1532108 — Re: [RFC] kernel/sysctl.c: return -EINVAL when write invalid val to ulong type sysctl

FromYisheng Xie <xieyisheng1@huawei.com>
Date2016-11-29 11:30 +0100
SubjectRe: [RFC] kernel/sysctl.c: return -EINVAL when write invalid val to ulong type sysctl
Message-ID<sINPI-7eO-17@gated-at.bofh.it>
In reply to#1530799

On 2016/11/27 7:15, subashab@codeaurora.org wrote:
> On 2016-11-26 02:13, Yisheng Xie wrote:
>> ----------------------
>> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
>> index 706309f..40e9285 100644
>> --- a/kernel/sysctl.c
>> +++ b/kernel/sysctl.c
>> @@ -2485,10 +2485,14 @@ static int __do_proc_doulongvec_minmax(void
>> *data, struct ctl_table *table, int
>>                                              sizeof(proc_wspace_sep), NULL);
>>                         if (err)
>>                                 break;
>> -                       if (neg)
>> -                               continue;
>> -                       if ((min && val < *min) || (max && val > *max))
>> -                               continue;
>> +                       if (neg) {
>> +                               err = -EINVAL;
>> +                               break;
>> +                       }
>> +                       if ((min && val < *min) || (max && val > *max)) {
>> +                               err = -EINVAL;
>> +                               break;
>> +                       }
>>                         *i = val;
>>                 } else {
>>                         val = convdiv * (*i) / convmul;
> 
> Agree, this should be similar to proc_douintvec
> 
> root@vm:~# echo 8192 > /proc/sys/net/core/xfrm_aevent_rseqth
> root@vm:~# cat /proc/sys/net/core/xfrm_aevent_rseqth
> 8192
> root@vm:~# echo -1 > /proc/sys/net/core/xfrm_aevent_rseqth
> -bash: echo: write error: Invalid argument
> root@vm:~# cat /proc/sys/net/core/xfrm_aevent_rseqth
> 8192
> 
Hi,
Thank you for your reply,
then I will write a formal patch to see whether it is acceptable.

Thanks,
Yisheng Xie
> -- 
> Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
> 
> .
> 

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web