| 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: <20101007071859.GD5471@cr0.nay.redhat.com>
Date: Thu, 7 Oct 2010 15:18:59 +0800
From: Américo Wang <xiyou.wangcong@...il.com>
To: Américo Wang <xiyou.wangcong@...il.com>
Cc: Eric Dumazet <eric.dumazet@...il.com>, Robin Holt <holt@....com>,
Andrew Morton <akpm@...ux-foundation.org>,
linux-kernel <linux-kernel@...r.kernel.org>,
Willy Tarreau <w@....eu>,
"David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org,
James Morris <jmorris@...ei.org>,
"Pekka Savola (ipv6)" <pekkas@...core.fi>,
Patrick McHardy <kaber@...sh.net>,
Alexey Kuznetsov <kuznet@....inr.ac.ru>, ebiederm@...ssion.com
Subject: Re: [PATCH] sysctl: fix min/max handling in
__do_proc_doulongvec_minmax()
On Tue, Oct 05, 2010 at 09:01:17PM +0800, Américo Wang wrote:
>On Mon, Oct 04, 2010 at 12:38:21PM +0200, Eric Dumazet wrote:
>>Le lundi 04 octobre 2010 à 18:35 +0800, Américo Wang a écrit :
>>
>>> Your patch does fix the problem, but seems not a good solution,
>>> we should skip all min/max checking if ->extra(1|2) is NULL,
>>> instead of checking it every time within the loop.
>>
>>Please do submit a patch, we'll see if you come to a better solution,
>>with no added code size (this is slow path, I dont care for checking it
>>'every time winthin the loop')
>>
>>
>
>I have one, but just did compile test. :)
>I will test it tomorrow.
>
Here is the final one.
--------------->
Eric D. noticed that we may trigger an OOPS if we leave ->extra{1,2}
to NULL when we use proc_doulongvec_minmax().
Actually, we don't need to store min/max values in a vector,
because all the elements in the vector should share the same min/max
value, like what proc_dointvec_minmax() does.
This also shrinks the binary size a little bit.
text data bss dec hex filename
12419 8744 4016 25179 625b kernel/sysctl.o.BEFORE
12395 8744 4024 25163 624b kernel/sysctl.o.AFTER
Reported-by: Eric Dumazet <eric.dumazet@...il.com>
Cc: Eric W. <ebiederm@...ssion.com>
Signed-off-by: WANG Cong <xiyou.wangcong@...il.com>
---
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index f88552c..ba5e511 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -2448,86 +2448,119 @@ int proc_dointvec_minmax(struct ctl_table *table, int write,
do_proc_dointvec_minmax_conv, ¶m);
}
-static int __do_proc_doulongvec_minmax(void *data, struct ctl_table *table, int write,
- void __user *buffer,
+static int __doulongvec_minmax_read(void *data, void __user *buffer,
size_t *lenp, loff_t *ppos,
unsigned long convmul,
unsigned long convdiv)
{
- unsigned long *i, *min, *max;
- int vleft, first = 1, err = 0;
- unsigned long page = 0;
- size_t left;
- char *kbuf;
+ unsigned long *i = (unsigned long *) data;
+ int err = 0;
+ bool first = true;
+ size_t left = *lenp;
- if (!data || !table->maxlen || !*lenp || (*ppos && !write)) {
- *lenp = 0;
- return 0;
+ for (; left; i++, first=false) {
+ unsigned long val;
+
+ val = convdiv * (*i) / convmul;
+ if (!first)
+ err = proc_put_char(&buffer, &left, '\t');
+ err = proc_put_long(&buffer, &left, val, false);
+ if (err)
+ break;
}
- i = (unsigned long *) data;
- min = (unsigned long *) table->extra1;
- max = (unsigned long *) table->extra2;
- vleft = table->maxlen / sizeof(unsigned long);
- left = *lenp;
+ if (!first && left && !err)
+ err = proc_put_char(&buffer, &left, '\n');
- if (write) {
- if (left > PAGE_SIZE - 1)
- left = PAGE_SIZE - 1;
- page = __get_free_page(GFP_TEMPORARY);
- kbuf = (char *) page;
- if (!kbuf)
- return -ENOMEM;
- if (copy_from_user(kbuf, buffer, left)) {
- err = -EFAULT;
- goto free;
- }
- kbuf[left] = 0;
+ *lenp -= left;
+ *ppos += *lenp;
+ return err;
+}
+
+static int __doulongvec_minmax_write(void *data, void __user *buffer,
+ size_t *lenp, loff_t *ppos, int vleft,
+ unsigned long min, unsigned long max)
+{
+ char *kbuf;
+ size_t left = *lenp;
+ unsigned long page = 0;
+ unsigned long *i = (unsigned long *) data;
+ int err = 0;
+ bool first = true;
+
+ if (left > PAGE_SIZE - 1)
+ left = PAGE_SIZE - 1;
+ page = __get_free_page(GFP_TEMPORARY);
+ kbuf = (char *) page;
+ if (!kbuf)
+ return -ENOMEM;
+ if (copy_from_user(kbuf, buffer, left)) {
+ err = -EFAULT;
+ goto free;
}
+ kbuf[left] = 0;
- for (; left && vleft--; i++, min++, max++, first=0) {
+ for (; left && vleft--; i++, first=false) {
unsigned long val;
+ bool neg;
- if (write) {
- bool neg;
-
- left -= proc_skip_spaces(&kbuf);
+ left -= proc_skip_spaces(&kbuf);
- err = proc_get_long(&kbuf, &left, &val, &neg,
- proc_wspace_sep,
- sizeof(proc_wspace_sep), NULL);
- if (err)
- break;
- if (neg)
- continue;
- if ((min && val < *min) || (max && val > *max))
- continue;
- *i = val;
- } else {
- val = convdiv * (*i) / convmul;
- if (!first)
- err = proc_put_char(&buffer, &left, '\t');
- err = proc_put_long(&buffer, &left, val, false);
- if (err)
- break;
- }
+ err = proc_get_long(&kbuf, &left, &val, &neg,
+ proc_wspace_sep,
+ sizeof(proc_wspace_sep), NULL);
+ if (err)
+ break;
+ if (neg)
+ continue;
+ if (val < min || val > max)
+ continue;
+ *i = val;
}
- if (!write && !first && left && !err)
- err = proc_put_char(&buffer, &left, '\n');
- if (write && !err)
+ if (!err)
left -= proc_skip_spaces(&kbuf);
free:
- if (write) {
- free_page(page);
- if (first)
- return err ? : -EINVAL;
- }
+ free_page(page);
+ if (first)
+ return err ? : -EINVAL;
+
*lenp -= left;
*ppos += *lenp;
return err;
}
+static int __do_proc_doulongvec_minmax(void *data, struct ctl_table *table, int write,
+ void __user *buffer,
+ size_t *lenp, loff_t *ppos,
+ unsigned long convmul,
+ unsigned long convdiv)
+{
+ if (!data || !table->maxlen || !*lenp || (*ppos && !write)) {
+ *lenp = 0;
+ return 0;
+ }
+
+ if (write) {
+ unsigned long min, max;
+ int vleft;
+
+ vleft = table->maxlen / sizeof(unsigned long);
+ if (table->extra1)
+ min = *(unsigned long *) table->extra1;
+ else
+ min = 0;
+ if (table->extra2)
+ max = *(unsigned long *) table->extra2;
+ else
+ max = ULONG_MAX;
+ return __doulongvec_minmax_write(data, buffer, lenp,
+ ppos, vleft, min, max);
+ } else
+ return __doulongvec_minmax_read(data, buffer, lenp,
+ ppos, convmul, convdiv);
+}
+
static int do_proc_doulongvec_minmax(struct ctl_table *table, int write,
void __user *buffer,
size_t *lenp, loff_t *ppos,
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists