* [PATCH 0/3] sysctl: fix some bugs related to proc_douintvec
@ 2017-04-07 15:51 Liping Zhang
2017-04-07 15:51 ` [PATCH 1/3] sysctl: add sanity check for proc_douintvec Liping Zhang
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Liping Zhang @ 2017-04-07 15:51 UTC (permalink / raw)
To: akpm, mingo, ebiederm
Cc: davem, subashab, torvalds, linux-kernel, Liping Zhang
From: Liping Zhang <zlpnobody@gmail.com>
This patch set aims to fix some bugs introduced by commit e7d316a02f68
("sysctl: handle error writing UINT_MAX to u32 fields").
Liping Zhang (3):
sysctl: add sanity check for proc_douintvec
sysctl: don't print negative flag for proc_douintvec
sysctl: report EINVAL if value is larger than UINT_MAX for
proc_douintvec
fs/proc/proc_sysctl.c | 1 +
kernel/sysctl.c | 3 +++
2 files changed, 4 insertions(+)
--
2.5.5
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/3] sysctl: add sanity check for proc_douintvec
2017-04-07 15:51 [PATCH 0/3] sysctl: fix some bugs related to proc_douintvec Liping Zhang
@ 2017-04-07 15:51 ` Liping Zhang
2017-04-07 15:51 ` [PATCH 2/3] sysctl: don't print negative flag " Liping Zhang
2017-04-07 15:51 ` [PATCH 3/3] sysctl: report EINVAL if value is larger than UINT_MAX " Liping Zhang
2 siblings, 0 replies; 7+ messages in thread
From: Liping Zhang @ 2017-04-07 15:51 UTC (permalink / raw)
To: akpm, mingo, ebiederm
Cc: davem, subashab, torvalds, linux-kernel, Liping Zhang
From: Liping Zhang <zlpnobody@gmail.com>
Commit e7d316a02f68 ("sysctl: handle error writing UINT_MAX to u32 fields")
introduced the proc_douintvec helper function, but it forgot to add
the related sanity check when doing register_sysctl_table. So add it now.
Signed-off-by: Liping Zhang <zlpnobody@gmail.com>
---
fs/proc/proc_sysctl.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index 8f91ec6..d04ea43 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -1074,6 +1074,7 @@ static int sysctl_check_table(const char *path, struct ctl_table *table)
if ((table->proc_handler == proc_dostring) ||
(table->proc_handler == proc_dointvec) ||
+ (table->proc_handler == proc_douintvec) ||
(table->proc_handler == proc_dointvec_minmax) ||
(table->proc_handler == proc_dointvec_jiffies) ||
(table->proc_handler == proc_dointvec_userhz_jiffies) ||
--
2.5.5
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/3] sysctl: don't print negative flag for proc_douintvec
2017-04-07 15:51 [PATCH 0/3] sysctl: fix some bugs related to proc_douintvec Liping Zhang
2017-04-07 15:51 ` [PATCH 1/3] sysctl: add sanity check for proc_douintvec Liping Zhang
@ 2017-04-07 15:51 ` Liping Zhang
2017-04-07 15:51 ` [PATCH 3/3] sysctl: report EINVAL if value is larger than UINT_MAX " Liping Zhang
2 siblings, 0 replies; 7+ messages in thread
From: Liping Zhang @ 2017-04-07 15:51 UTC (permalink / raw)
To: akpm, mingo, ebiederm
Cc: davem, subashab, torvalds, linux-kernel, Liping Zhang
From: Liping Zhang <zlpnobody@gmail.com>
I saw some very confusing sysctl output on my system:
# cat /proc/sys/net/core/xfrm_aevent_rseqth
-2
# cat /proc/sys/net/core/xfrm_aevent_etime
-10
cat /proc/sys/net/ipv4/tcp_notsent_lowat
-4294967295
Because we forget to set the *negp flag in proc_douintvec, so it will
become a garbage value.
Since the value related to proc_douintvec is always an unsigned integer,
so we can set *negp to false explictily to fix this issue.
Fixes: e7d316a02f68 ("sysctl: handle error writing UINT_MAX to u32 fields")
Signed-off-by: Liping Zhang <zlpnobody@gmail.com>
---
kernel/sysctl.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index acf0a5a..8cca4c7 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -2136,6 +2136,7 @@ static int do_proc_douintvec_conv(bool *negp, unsigned long *lvalp,
*valp = *lvalp;
} else {
unsigned int val = *valp;
+ *negp = false;
*lvalp = (unsigned long)val;
}
return 0;
--
2.5.5
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/3] sysctl: report EINVAL if value is larger than UINT_MAX for proc_douintvec
2017-04-07 15:51 [PATCH 0/3] sysctl: fix some bugs related to proc_douintvec Liping Zhang
2017-04-07 15:51 ` [PATCH 1/3] sysctl: add sanity check for proc_douintvec Liping Zhang
2017-04-07 15:51 ` [PATCH 2/3] sysctl: don't print negative flag " Liping Zhang
@ 2017-04-07 15:51 ` Liping Zhang
2017-04-07 16:49 ` Linus Torvalds
2 siblings, 1 reply; 7+ messages in thread
From: Liping Zhang @ 2017-04-07 15:51 UTC (permalink / raw)
To: akpm, mingo, ebiederm
Cc: davem, subashab, torvalds, linux-kernel, Liping Zhang
From: Liping Zhang <zlpnobody@gmail.com>
Currently, inputting the following command will succeed but actually the
value will be truncated:
# echo 0x12ffffffff > /proc/sys/net/ipv4/tcp_notsent_lowat
This is not friendly to the user, so instead, we should report error
when the value is larger than UINT_MAX.
Fixes: e7d316a02f68 ("sysctl: handle error writing UINT_MAX to u32 fields")
Signed-off-by: Liping Zhang <zlpnobody@gmail.com>
---
kernel/sysctl.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 8cca4c7..e8d1ed0 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -2133,6 +2133,8 @@ static int do_proc_douintvec_conv(bool *negp, unsigned long *lvalp,
if (write) {
if (*negp)
return -EINVAL;
+ if (*lvalp > (unsigned long) UINT_MAX)
+ return -EINVAL;
*valp = *lvalp;
} else {
unsigned int val = *valp;
--
2.5.5
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] sysctl: report EINVAL if value is larger than UINT_MAX for proc_douintvec
2017-04-07 15:51 ` [PATCH 3/3] sysctl: report EINVAL if value is larger than UINT_MAX " Liping Zhang
@ 2017-04-07 16:49 ` Linus Torvalds
2017-04-07 21:01 ` Eric W. Biederman
0 siblings, 1 reply; 7+ messages in thread
From: Linus Torvalds @ 2017-04-07 16:49 UTC (permalink / raw)
To: Liping Zhang
Cc: Andrew Morton, Ingo Molnar, Eric W. Biederman, David Miller,
Subash Abhinov Kasiviswanathan, Linux Kernel Mailing List,
Liping Zhang
On Fri, Apr 7, 2017 at 8:51 AM, Liping Zhang <zlpnobody@163.com> wrote:
> From: Liping Zhang <zlpnobody@gmail.com>
>
> Currently, inputting the following command will succeed but actually the
> value will be truncated:
> # echo 0x12ffffffff > /proc/sys/net/ipv4/tcp_notsent_lowat
>
> This is not friendly to the user, so instead, we should report error
> when the value is larger than UINT_MAX.
I applied the two other patches, but I didn't apply this one.
It's entirely possible that people end up doing something like
echo -1 > /proc/sys/some_random_uint
because that's a fairly normal thing to do to set all bits. Making
that an error seems wrong.
Linus
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] sysctl: report EINVAL if value is larger than UINT_MAX for proc_douintvec
2017-04-07 16:49 ` Linus Torvalds
@ 2017-04-07 21:01 ` Eric W. Biederman
2017-04-08 17:21 ` Linus Torvalds
0 siblings, 1 reply; 7+ messages in thread
From: Eric W. Biederman @ 2017-04-07 21:01 UTC (permalink / raw)
To: Linus Torvalds
Cc: Liping Zhang, Andrew Morton, Ingo Molnar, David Miller,
Subash Abhinov Kasiviswanathan, Linux Kernel Mailing List,
Liping Zhang
Linus Torvalds <torvalds@linux-foundation.org> writes:
> On Fri, Apr 7, 2017 at 8:51 AM, Liping Zhang <zlpnobody@163.com> wrote:
>> From: Liping Zhang <zlpnobody@gmail.com>
>>
>> Currently, inputting the following command will succeed but actually the
>> value will be truncated:
>> # echo 0x12ffffffff > /proc/sys/net/ipv4/tcp_notsent_lowat
>>
>> This is not friendly to the user, so instead, we should report error
>> when the value is larger than UINT_MAX.
>
> I applied the two other patches, but I didn't apply this one.
>
> It's entirely possible that people end up doing something like
>
> echo -1 > /proc/sys/some_random_uint
>
> because that's a fairly normal thing to do to set all bits. Making
> that an error seems wrong.
Except that doesn't help in this case. The function do_uintvec_conv
rules already rejects all negative values on write. So -1 is already
rejected.
In fact the function proc_douintvec_conv has always rejected negative
values so this change won't even create a regression.
So it looks perfectly reasonable to reject values that are simply too
large to be written to the uint.
So even today to write all bits set you do have to do:
echo 0xffffffff > /proc/sys/some_random_uint
Eric
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] sysctl: report EINVAL if value is larger than UINT_MAX for proc_douintvec
2017-04-07 21:01 ` Eric W. Biederman
@ 2017-04-08 17:21 ` Linus Torvalds
0 siblings, 0 replies; 7+ messages in thread
From: Linus Torvalds @ 2017-04-08 17:21 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Liping Zhang, Andrew Morton, Ingo Molnar, David Miller,
Subash Abhinov Kasiviswanathan, Linux Kernel Mailing List,
Liping Zhang
On Fri, Apr 7, 2017 at 2:01 PM, Eric W. Biederman <ebiederm@xmission.com> wrote:
>
> Except that doesn't help in this case. The function do_uintvec_conv
> rules already rejects all negative values on write. So -1 is already
> rejected.
Oh, I should have noticed that.
So yes, that patch looks good too. Will apply.
Linus
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-04-08 17:22 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-07 15:51 [PATCH 0/3] sysctl: fix some bugs related to proc_douintvec Liping Zhang
2017-04-07 15:51 ` [PATCH 1/3] sysctl: add sanity check for proc_douintvec Liping Zhang
2017-04-07 15:51 ` [PATCH 2/3] sysctl: don't print negative flag " Liping Zhang
2017-04-07 15:51 ` [PATCH 3/3] sysctl: report EINVAL if value is larger than UINT_MAX " Liping Zhang
2017-04-07 16:49 ` Linus Torvalds
2017-04-07 21:01 ` Eric W. Biederman
2017-04-08 17:21 ` Linus Torvalds
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).