| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20090501145639.GA24443@linux>
Date: Fri, 1 May 2009 16:56:40 +0200
From: Andrea Righi <righi.andrea@...il.com>
To: Andrew Morton <akpm@...ux-foundation.org>
Cc: peterz@...radead.org, rientjes@...gle.com, david@...morbit.com,
cl@...ux-foundation.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mm: prevent divide error for small values of
vm_dirty_bytes
On Wed, Apr 29, 2009 at 02:46:55PM -0700, Andrew Morton wrote:
> On Wed, 29 Apr 2009 11:34:51 +0200
> Andrea Righi <righi.andrea@...il.com> wrote:
>
> > --- a/Documentation/sysctl/vm.txt
> > +++ b/Documentation/sysctl/vm.txt
> > @@ -90,6 +90,10 @@ will itself start writeback.
> > If dirty_bytes is written, dirty_ratio becomes a function of its value
> > (dirty_bytes / the amount of dirtyable system memory).
> >
> > +Note: the minimum value allowed for dirty_bytes is two pages (in bytes); any
> > +value lower than this limit will be ignored and the old configuration will be
> > +retained.
>
> Well. This implies that the write to the procfs file would appear to
> succeed. One hopes that the write would in fact return -EINVAL or
> such?
I definitely agree. Just tested the following patch and it looks much
better with the error code.
-Andrea
---
sysctl: return error code if values are not within a valid range
Currently __do_proc_doulongvec_minmax(), as well as
__do_proc_dointvec(), simply skip the invalid values instead of return
-EINVAL.
A more correct behaviour is to report to the userspace that some values
were invalid and they couldn't be written instead of silently drop
them.
For example (vm_dirty_bytes must be greater or equal than 2*PAGE_SIZE):
- before:
# cat /proc/sys/vm/dirty_bytes
0
# /bin/echo 1 > /proc/sys/vm/dirty_bytes
# cat /proc/sys/vm/dirty_bytes
0
# /bin/echo 8192 > /proc/sys/vm/dirty_bytes
# cat /proc/sys/vm/dirty_bytes
8192
- after:
# cat /proc/sys/vm/dirty_bytes
0
# /bin/echo 1 > /proc/sys/vm/dirty_bytes
/bin/echo: write error: Invalid argument
# cat /proc/sys/vm/dirty_bytes
0
# /bin/echo 8192 > /proc/sys/vm/dirty_bytes
# cat /proc/sys/vm/dirty_bytes
8192
As a bonus do few minor coding style fixup.
Signed-off-by: Andrea Righi <righi.andrea@...il.com>
---
kernel/sysctl.c | 47 +++++++++++++++++++++++++++++++----------------
1 files changed, 31 insertions(+), 16 deletions(-)
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index ea78fa1..92e56cf 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -2243,19 +2243,19 @@ static int __do_proc_dointvec(void *tbl_data, struct ctl_table *table,
void *data)
{
#define TMPBUFLEN 21
- int *i, vleft, first=1, neg, val;
+ int *i, vleft, first = 1, neg, val, ret = 0;
unsigned long lval;
size_t left, len;
-
+
char buf[TMPBUFLEN], *p;
char __user *s = buffer;
-
+
if (!tbl_data || !table->maxlen || !*lenp ||
(*ppos && !write)) {
*lenp = 0;
return 0;
}
-
+
i = (int *) tbl_data;
vleft = table->maxlen / sizeof(*i);
left = *lenp;
@@ -2288,26 +2288,31 @@ static int __do_proc_dointvec(void *tbl_data, struct ctl_table *table,
neg = 1;
p++;
}
- if (*p < '0' || *p > '9')
+ if (*p < '0' || *p > '9') {
+ ret = -EINVAL;
break;
+ }
lval = simple_strtoul(p, &p, 0);
len = p-buf;
- if ((len < left) && *p && !isspace(*p))
+ if ((len < left) && *p && !isspace(*p)) {
+ ret = -EINVAL;
break;
+ }
if (neg)
val = -val;
s += len;
left -= len;
- if (conv(&neg, &lval, i, 1, data))
+ ret = conv(&neg, &lval, i, 1, data);
+ if (ret)
break;
} else {
p = buf;
if (!first)
*p++ = '\t';
-
+
if (conv(&neg, &lval, i, 0, data))
break;
@@ -2339,6 +2344,8 @@ static int __do_proc_dointvec(void *tbl_data, struct ctl_table *table,
}
if (write && first)
return -EINVAL;
+ if (write && ret)
+ return ret;
*lenp -= left;
*ppos += *lenp;
return 0;
@@ -2477,23 +2484,23 @@ static int __do_proc_doulongvec_minmax(void *data, struct ctl_table *table, int
{
#define TMPBUFLEN 21
unsigned long *i, *min, *max, val;
- int vleft, first=1, neg;
+ int vleft, first = 1, neg, ret = 0;
size_t len, left;
char buf[TMPBUFLEN], *p;
char __user *s = buffer;
-
+
if (!data || !table->maxlen || !*lenp ||
(*ppos && !write)) {
*lenp = 0;
return 0;
}
-
+
i = (unsigned long *) data;
min = (unsigned long *) table->extra1;
max = (unsigned long *) table->extra2;
vleft = table->maxlen / sizeof(unsigned long);
left = *lenp;
-
+
for (; left && vleft--; i++, min++, max++, first=0) {
if (write) {
while (left) {
@@ -2519,12 +2526,16 @@ static int __do_proc_doulongvec_minmax(void *data, struct ctl_table *table, int
neg = 1;
p++;
}
- if (*p < '0' || *p > '9')
+ if (*p < '0' || *p > '9') {
+ ret = -EINVAL;
break;
+ }
val = simple_strtoul(p, &p, 0) * convmul / convdiv ;
len = p-buf;
- if ((len < left) && *p && !isspace(*p))
+ if ((len < left) && *p && !isspace(*p)) {
+ ret = -EINVAL;
break;
+ }
if (neg)
val = -val;
s += len;
@@ -2532,8 +2543,10 @@ static int __do_proc_doulongvec_minmax(void *data, struct ctl_table *table, int
if(neg)
continue;
- if ((min && val < *min) || (max && val > *max))
- continue;
+ if ((min && val < *min) || (max && val > *max)) {
+ ret = -EINVAL;
+ break;
+ }
*i = val;
} else {
p = buf;
@@ -2567,6 +2580,8 @@ static int __do_proc_doulongvec_minmax(void *data, struct ctl_table *table, int
}
if (write && first)
return -EINVAL;
+ if (write && ret)
+ return ret;
*lenp -= left;
*ppos += *lenp;
return 0;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists