* [RFC] kernel/sysctl.c: return -EINVAL when write invalid val to ulong type sysctl
@ 2016-11-26 9:13 Yisheng Xie
2016-11-26 23:15 ` subashab
0 siblings, 1 reply; 3+ messages in thread
From: Yisheng Xie @ 2016-11-26 9:13 UTC (permalink / raw)
To: Linux Kernel Mailing List
Cc: Andrew Morton, Arnaldo Carvalho de Melo, Mel Gorman, Al Viro,
Johannes Weiner, Eric W. Biederman, Daniel Bristot de Oliveira,
Subash Abhinov Kasiviswanathan, Daniel Cashman, Willy Tarreau,
Arnd Bergmann, Hanjun Guo, Xishi Qiu
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;
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [RFC] kernel/sysctl.c: return -EINVAL when write invalid val to ulong type sysctl
2016-11-26 9:13 [RFC] kernel/sysctl.c: return -EINVAL when write invalid val to ulong type sysctl Yisheng Xie
@ 2016-11-26 23:15 ` subashab
2016-11-29 10:24 ` Yisheng Xie
0 siblings, 1 reply; 3+ messages in thread
From: subashab @ 2016-11-26 23:15 UTC (permalink / raw)
To: Yisheng Xie
Cc: Linux Kernel Mailing List, Andrew Morton,
Arnaldo Carvalho de Melo, Mel Gorman, Al Viro, Johannes Weiner,
Eric W. Biederman, Daniel Bristot de Oliveira, Daniel Cashman,
Willy Tarreau, Arnd Bergmann, Hanjun Guo, Xishi Qiu
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [RFC] kernel/sysctl.c: return -EINVAL when write invalid val to ulong type sysctl
2016-11-26 23:15 ` subashab
@ 2016-11-29 10:24 ` Yisheng Xie
0 siblings, 0 replies; 3+ messages in thread
From: Yisheng Xie @ 2016-11-29 10:24 UTC (permalink / raw)
To: subashab
Cc: Linux Kernel Mailing List, Andrew Morton,
Arnaldo Carvalho de Melo, Mel Gorman, Al Viro, Johannes Weiner,
Eric W. Biederman, Daniel Bristot de Oliveira, Daniel Cashman,
Willy Tarreau, Arnd Bergmann, Hanjun Guo, Xishi Qiu
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
>
> .
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-11-29 10:26 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-26 9:13 [RFC] kernel/sysctl.c: return -EINVAL when write invalid val to ulong type sysctl Yisheng Xie
2016-11-26 23:15 ` subashab
2016-11-29 10:24 ` Yisheng Xie
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).