From patchwork Thu Oct 7 09:25:38 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Cong Wang X-Patchwork-Id: 67016 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id CA470B6EEA for ; Thu, 7 Oct 2010 20:21:50 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1760118Ab0JGJVI (ORCPT ); Thu, 7 Oct 2010 05:21:08 -0400 Received: from mail-pv0-f174.google.com ([74.125.83.174]:35467 "EHLO mail-pv0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753695Ab0JGJVF (ORCPT ); Thu, 7 Oct 2010 05:21:05 -0400 Received: by pvg2 with SMTP id 2so94141pvg.19 for ; Thu, 07 Oct 2010 02:21:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:date:from:to:cc:subject :message-id:references:mime-version:content-type:content-disposition :in-reply-to:user-agent; bh=GaGmwNchdLEC1ChSUyyWRXbKTefI9UfoEONQvCBqZ8k=; b=X3EvBC/47Cy/LeIwH6Do2rKDT9fyb1vMMMuCNmiJt10wmBK4qzD6lkkK70K0YUyBnp L7Cn5wgpBmwrld+Cui7LH/ZhEBYs2K3GsamaUDP9lGHfTILIOawipTRSpvEfcjXm0vOH ZqdkcekUn611cMGfXp0A9QXxaes0SOO7DJLK8= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; b=AowoXQkPjyWX6MsV836BVG0he6+XNLkMDdv0J7raHU3dFloh9P+GhU/TEg+FxhBz0u g+OCl6jAzWUXfMRzMrwSCwFLn8evC1YUQ/Ag1N2VwhB7Mf8udIlk+wtxXu2s5/3jyOy7 1XAr74NR9iGng/9r3hqCEI8SoaL5j4x6PSOd8= Received: by 10.142.48.17 with SMTP id v17mr366045wfv.233.1286443265261; Thu, 07 Oct 2010 02:21:05 -0700 (PDT) Received: from cr0.nay.redhat.com ([60.247.97.98]) by mx.google.com with ESMTPS id t38sm2046812wfc.21.2010.10.07.02.21.01 (version=TLSv1/SSLv3 cipher=RC4-MD5); Thu, 07 Oct 2010 02:21:04 -0700 (PDT) Date: Thu, 7 Oct 2010 17:25:38 +0800 From: =?utf-8?Q?Am=C3=A9rico?= Wang To: =?utf-8?Q?Am=C3=A9rico?= Wang Cc: Eric Dumazet , Robin Holt , Andrew Morton , linux-kernel , Willy Tarreau , "David S. Miller" , netdev@vger.kernel.org, James Morris , "Pekka Savola (ipv6)" , Patrick McHardy , Alexey Kuznetsov , ebiederm@xmission.com Subject: [PATCH] sysctl: fix min/max handling in __do_proc_doulongvec_minmax() Message-ID: <20101007092538.GE5471@cr0.nay.redhat.com> References: <1286025469.2582.1806.camel@edumazet-laptop> <20101004085913.GR14068@sgi.com> <1286183058.18293.26.camel@edumazet-laptop> <20101004093439.GG5189@cr0.nay.redhat.com> <1286187030.18293.33.camel@edumazet-laptop> <20101004103545.GJ5189@cr0.nay.redhat.com> <1286188701.18293.57.camel@edumazet-laptop> <20101005130117.GK5170@cr0.nay.redhat.com> <20101007071859.GD5471@cr0.nay.redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20101007071859.GD5471@cr0.nay.redhat.com> User-Agent: Mutt/1.5.20 (2009-06-14) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org >> > >Here is the final one. Oops, that one is not correct. Hopefully this one is correct. ---------------> 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. Reported-by: Eric Dumazet Cc: Eric W. Signed-off-by: WANG Cong --- -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/kernel/sysctl.c b/kernel/sysctl.c index f88552c..fad9208 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, - size_t *lenp, loff_t *ppos, +static int __doulongvec_minmax_read(void *data, void __user *buffer, + size_t *lenp, loff_t *ppos, int vleft, 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 = data; + int err = 0; + bool first = true; + size_t left = *lenp; - if (!data || !table->maxlen || !*lenp || (*ppos && !write)) { - *lenp = 0; - return 0; + for (; left && vleft--; 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) +{ + int vleft; + if (!data || !table->maxlen || !*lenp || (*ppos && !write)) { + *lenp = 0; + return 0; + } + + vleft = table->maxlen / sizeof(unsigned long); + if (write) { + unsigned long min, max; + + 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, vleft, convmul, convdiv); +} + static int do_proc_doulongvec_minmax(struct ctl_table *table, int write, void __user *buffer, size_t *lenp, loff_t *ppos,