* [PATCH] kernel/sysctl: return -EINVAL if write invalid val to ulong type sysctl
@ 2016-11-30 10:30 Yisheng Xie
2016-11-30 21:33 ` Andrew Morton
0 siblings, 1 reply; 6+ messages in thread
From: Yisheng Xie @ 2016-11-30 10:30 UTC (permalink / raw)
To: linux-kernel
Cc: akpm, acme, mgorman, viro, hannes, ebiederm, bristot, subashab,
dcashman, w, arnd, guohanjun, qiuxishi
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 checking
the value again.
This patch return -EINVAL when write an invalid value to unsigned
long type sysctl to make user know what happened without
checking its value once more, just as what proc_douintvec do.
Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
---
This is a patchset after RFC, you can see the former discussion at
https://lkml.org/lkml/2016/11/26/48
Any comment is more than welcome.
Thanks,
Yisheng Xie.
---
kernel/sysctl.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
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;
--
1.7.12.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] kernel/sysctl: return -EINVAL if write invalid val to ulong type sysctl
2016-11-30 10:30 [PATCH] kernel/sysctl: return -EINVAL if write invalid val to ulong type sysctl Yisheng Xie
@ 2016-11-30 21:33 ` Andrew Morton
2016-12-02 6:54 ` Yisheng Xie
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2016-11-30 21:33 UTC (permalink / raw)
To: Yisheng Xie
Cc: linux-kernel, acme, mgorman, viro, hannes, ebiederm, bristot,
subashab, dcashman, w, arnd, guohanjun, qiuxishi
On Wed, 30 Nov 2016 18:30:52 +0800 Yisheng Xie <xieyisheng1@huawei.com> 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 checking
> the value again.
>
> This patch return -EINVAL when write an invalid value to unsigned
> long type sysctl to make user know what happened without
> checking its value once more, just as what proc_douintvec do.
hmpf.
# echo 18446744073709551615 > /proc/sys/vm/user_reserve_kbytes
# cat /proc/sys/vm/user_reserve_kbytes
18446744073709551615
I think that when taking in an unsigned long the kernel should simply
treat -1 as 0xffffffff (or 0xffffffffffffffff). It's natural and
normal and everyone knows what it means?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] kernel/sysctl: return -EINVAL if write invalid val to ulong type sysctl
2016-11-30 21:33 ` Andrew Morton
@ 2016-12-02 6:54 ` Yisheng Xie
2016-12-02 19:24 ` Eric W. Biederman
0 siblings, 1 reply; 6+ messages in thread
From: Yisheng Xie @ 2016-12-02 6:54 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-kernel, acme, mgorman, viro, hannes, ebiederm, bristot,
subashab, dcashman, w, arnd, guohanjun, qiuxishi
On 2016/12/1 5:33, Andrew Morton wrote:
> On Wed, 30 Nov 2016 18:30:52 +0800 Yisheng Xie <xieyisheng1@huawei.com> 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 checking
>> the value again.
>>
>> This patch return -EINVAL when write an invalid value to unsigned
>> long type sysctl to make user know what happened without
>> checking its value once more, just as what proc_douintvec do.
>
> hmpf.
>
> # echo 18446744073709551615 > /proc/sys/vm/user_reserve_kbytes
> # cat /proc/sys/vm/user_reserve_kbytes
> 18446744073709551615
>
> I think that when taking in an unsigned long the kernel should simply
> treat -1 as 0xffffffff (or 0xffffffffffffffff). It's natural and
> normal and everyone knows what it means?
>
Hi Andrew,
Thank you for your reply.
Do you means it should be like this:
# echo -1 > /proc/sys/vm/user_reserve_kbytes
# cat /proc/sys/vm/user_reserve_kbytes
18446744073709551615
I looks ok to me, however, I not sure whether other code in the kernel
will also use its complement if user write a negative number for an
unsigned long. Does anyone have other opinion ?
Thanks
Yisheng Xie.
>
>
>
> .
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] kernel/sysctl: return -EINVAL if write invalid val to ulong type sysctl
2016-12-02 6:54 ` Yisheng Xie
@ 2016-12-02 19:24 ` Eric W. Biederman
2016-12-05 6:12 ` Xishi Qiu
2016-12-06 12:50 ` Yisheng Xie
0 siblings, 2 replies; 6+ messages in thread
From: Eric W. Biederman @ 2016-12-02 19:24 UTC (permalink / raw)
To: Yisheng Xie
Cc: Andrew Morton, linux-kernel, acme, mgorman, viro, hannes, bristot,
subashab, dcashman, w, arnd, guohanjun, qiuxishi
Yisheng Xie <xieyisheng1@huawei.com> writes:
> On 2016/12/1 5:33, Andrew Morton wrote:
>> On Wed, 30 Nov 2016 18:30:52 +0800 Yisheng Xie <xieyisheng1@huawei.com> 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 checking
>>> the value again.
>>>
>>> This patch return -EINVAL when write an invalid value to unsigned
>>> long type sysctl to make user know what happened without
>>> checking its value once more, just as what proc_douintvec do.
>>
>> hmpf.
>>
>> # echo 18446744073709551615 > /proc/sys/vm/user_reserve_kbytes
>> # cat /proc/sys/vm/user_reserve_kbytes
>> 18446744073709551615
>>
>> I think that when taking in an unsigned long the kernel should simply
>> treat -1 as 0xffffffff (or 0xffffffffffffffff). It's natural and
>> normal and everyone knows what it means?
>>
> Hi Andrew,
> Thank you for your reply.
> Do you means it should be like this:
> # echo -1 > /proc/sys/vm/user_reserve_kbytes
> # cat /proc/sys/vm/user_reserve_kbytes
> 18446744073709551615
>
> I looks ok to me, however, I not sure whether other code in the kernel
> will also use its complement if user write a negative number for an
> unsigned long. Does anyone have other opinion ?
Largely we need to be very careful with changing these functions as
they have been around for a long time, and have a very diverse set of
users.
So while changes are possible a reasonable argument needs to be made
that nothing in userspace cares.
Eric
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] kernel/sysctl: return -EINVAL if write invalid val to ulong type sysctl
2016-12-02 19:24 ` Eric W. Biederman
@ 2016-12-05 6:12 ` Xishi Qiu
2016-12-06 12:50 ` Yisheng Xie
1 sibling, 0 replies; 6+ messages in thread
From: Xishi Qiu @ 2016-12-05 6:12 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Yisheng Xie, Andrew Morton, linux-kernel, acme, mgorman, viro,
hannes, bristot, subashab, dcashman, w, arnd, guohanjun
On 2016/12/3 3:24, Eric W. Biederman wrote:
> Yisheng Xie <xieyisheng1@huawei.com> writes:
>
>> On 2016/12/1 5:33, Andrew Morton wrote:
>>> On Wed, 30 Nov 2016 18:30:52 +0800 Yisheng Xie <xieyisheng1@huawei.com> 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 checking
>>>> the value again.
>>>>
>>>> This patch return -EINVAL when write an invalid value to unsigned
>>>> long type sysctl to make user know what happened without
>>>> checking its value once more, just as what proc_douintvec do.
>>>
>>> hmpf.
>>>
>>> # echo 18446744073709551615 > /proc/sys/vm/user_reserve_kbytes
>>> # cat /proc/sys/vm/user_reserve_kbytes
>>> 18446744073709551615
>>>
>>> I think that when taking in an unsigned long the kernel should simply
>>> treat -1 as 0xffffffff (or 0xffffffffffffffff). It's natural and
>>> normal and everyone knows what it means?
>>>
>> Hi Andrew,
>> Thank you for your reply.
>> Do you means it should be like this:
>> # echo -1 > /proc/sys/vm/user_reserve_kbytes
>> # cat /proc/sys/vm/user_reserve_kbytes
>> 18446744073709551615
>>
>> I looks ok to me, however, I not sure whether other code in the kernel
>> will also use its complement if user write a negative number for an
>> unsigned long. Does anyone have other opinion ?
>
> Largely we need to be very careful with changing these functions as
> they have been around for a long time, and have a very diverse set of
> users.
>
> So while changes are possible a reasonable argument needs to be made
> that nothing in userspace cares.
>
> Eric
>
Hi Eric,
This patch is aimed to change the return value if write invalid value to
ulong type sysctl, just to keep the same as int type sysctl.
Thanks,
Xishi Qiu
> .
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] kernel/sysctl: return -EINVAL if write invalid val to ulong type sysctl
2016-12-02 19:24 ` Eric W. Biederman
2016-12-05 6:12 ` Xishi Qiu
@ 2016-12-06 12:50 ` Yisheng Xie
1 sibling, 0 replies; 6+ messages in thread
From: Yisheng Xie @ 2016-12-06 12:50 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Andrew Morton, linux-kernel, acme, mgorman, viro, hannes, bristot,
subashab, dcashman, w, arnd, guohanjun, qiuxishi
On 2016/12/3 3:24, Eric W. Biederman wrote:
> Yisheng Xie <xieyisheng1@huawei.com> writes:
>
>> On 2016/12/1 5:33, Andrew Morton wrote:
>>> On Wed, 30 Nov 2016 18:30:52 +0800 Yisheng Xie <xieyisheng1@huawei.com> 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 checking
>>>> the value again.
>>>>
>>>> This patch return -EINVAL when write an invalid value to unsigned
>>>> long type sysctl to make user know what happened without
>>>> checking its value once more, just as what proc_douintvec do.
>>>
>>> hmpf.
>>>
>>> # echo 18446744073709551615 > /proc/sys/vm/user_reserve_kbytes
>>> # cat /proc/sys/vm/user_reserve_kbytes
>>> 18446744073709551615
>>>
>>> I think that when taking in an unsigned long the kernel should simply
>>> treat -1 as 0xffffffff (or 0xffffffffffffffff). It's natural and
>>> normal and everyone knows what it means?
>>>
>> Hi Andrew,
>> Thank you for your reply.
>> Do you means it should be like this:
>> # echo -1 > /proc/sys/vm/user_reserve_kbytes
>> # cat /proc/sys/vm/user_reserve_kbytes
>> 18446744073709551615
>>
>> I looks ok to me, however, I not sure whether other code in the kernel
>> will also use its complement if user write a negative number for an
>> unsigned long. Does anyone have other opinion ?
>
> Largely we need to be very careful with changing these functions as
> they have been around for a long time, and have a very diverse set of
> users.
Hi Eric,
Thanks for your reply.
That right.
>
> So while changes are possible a reasonable argument needs to be made
> that nothing in userspace cares.
>
So the patch's original aim that return -EINVAL when user write
invalid val to ulong type sysctl is more reasonable, Right?
Thanks,
Yisheng Xie
> Eric
>
> .
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-12-06 12:56 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-11-30 10:30 [PATCH] kernel/sysctl: return -EINVAL if write invalid val to ulong type sysctl Yisheng Xie
2016-11-30 21:33 ` Andrew Morton
2016-12-02 6:54 ` Yisheng Xie
2016-12-02 19:24 ` Eric W. Biederman
2016-12-05 6:12 ` Xishi Qiu
2016-12-06 12:50 ` Yisheng Xie
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox