All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg KH <gregkh@suse.de>
To: linux-kernel@vger.kernel.org, stable@kernel.org
Cc: Justin Forbes <jmforbes-a5Mqy2EUIFVAfugRpC6u6w@public.gmane.org>,
	Zwane Mwaikambo <zwane@arm.linux.org.uk>,
	Theodore Ts'o <tytso@mit.edu>,
	Randy Dunlap <rdunlap@xenotime.net>,
	Dave Jones <davej@redhat.com>,
	Chuck Wolber <chuckw-lrwSPXAIdEw7YuNMryXyOw@public.gmane.org>,
	Chris Wedgwood <reviews-vFB7bnJMxjixIvdkapsUdQ@public.gmane.org>,
	Michael Krufky <mkrufky-dJidKbW2IEtAfugRpC6u6w@public.gmane.org>,
	Chuck Ebbert <cebbert@redhat.com>,
	Domenico Andreoli <cavokz@gmail.com>, Willy Tarreau <w@1wt.eu>,
	Rodrigo Rubira Branco
	<rbranco-OG872enO0lhw4vQD1pnt9QC/G2K4zDHf@public.gmane.org>,
	Jake Edge <jake-T1hC0tSOHrs@public.gmane.org>,
	Eugene Teo <eteo@redhat.com>,
	torvalds@linux-foundation.org, akpm@linux-foundation.org,
	alan@lxorguk.ukuu.org.uk, linux-nfs@vger.kernel.org,
	Greg Banks <gnb@sgi.com>, Neil Brown <neilb@suse.de>,
	"J. Bruce Fields" <bfields@citi.umich.edu>,
	Tom Tucker <tom@opengridcomputing.com>,
	Ingo Oeser <ioe-lkml-MFZNYGWX9eyELgA04lAiVw@public.gmane.org>,
	Cyrill Gorcunov <gorcunov@gmail.com>,
	Chuck Lever <chuck.lever@oracle.com>
Subject: [patch 03/16] sunrpc: fix possible overrun on read of /proc/sys/sunrpc/transports
Date: Wed, 3 Sep 2008 10:32:47 -0700	[thread overview]
Message-ID: <20080903173247.GD10429@suse.de> (raw)
In-Reply-To: <20080903173218.GA10429@suse.de>


2.6.25-stable review patch.  If anyone has any objections, please let us know.

------------------
From: Cyrill Gorcunov <gorcunov@gmail.com>

commit 27df6f25ff218072e0e879a96beeb398a79cdbc8 upstream

Vegard Nossum reported
----------------------
> I noticed that something weird is going on with /proc/sys/sunrpc/transports.
> This file is generated in net/sunrpc/sysctl.c, function proc_do_xprt(). When
> I "cat" this file, I get the expected output:
>    $ cat /proc/sys/sunrpc/transports
>    tcp 1048576
>    udp 32768

> But I think that it does not check the length of the buffer supplied by
> userspace to read(). With my original program, I found that the stack was
> being overwritten by the characters above, even when the length given to
> read() was just 1.

David Wagner added (among other things) that copy_to_user could be
probably used here.

Ingo Oeser suggested to use simple_read_from_buffer() here.

The conclusion is that proc_do_xprt doesn't check for userside buffer
size indeed so fix this by using Ingo's suggestion.

Reported-by: Vegard Nossum <vegard.nossum@gmail.com>
Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
CC: Ingo Oeser <ioe-lkml-MFZNYGWX9eyELgA04lAiVw@public.gmane.org>
Cc: Neil Brown <neilb@suse.de>
Cc: Chuck Lever <chuck.lever@oracle.com>
Cc: Greg Banks <gnb@sgi.com>
Cc: Tom Tucker <tom@opengridcomputing.com>
Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

---
 net/sunrpc/sysctl.c |   18 ++++--------------
 1 file changed, 4 insertions(+), 14 deletions(-)

--- a/net/sunrpc/sysctl.c
+++ b/net/sunrpc/sysctl.c
@@ -60,24 +60,14 @@ static int proc_do_xprt(ctl_table *table
 			void __user *buffer, size_t *lenp, loff_t *ppos)
 {
 	char tmpbuf[256];
-	int len;
+	size_t len;
+
 	if ((*ppos && !write) || !*lenp) {
 		*lenp = 0;
 		return 0;
 	}
-	if (write)
-		return -EINVAL;
-	else {
-		len = svc_print_xprts(tmpbuf, sizeof(tmpbuf));
-		if (!access_ok(VERIFY_WRITE, buffer, len))
-			return -EFAULT;
-
-		if (__copy_to_user(buffer, tmpbuf, len))
-			return -EFAULT;
-	}
-	*lenp -= len;
-	*ppos += len;
-	return 0;
+	len = svc_print_xprts(tmpbuf, sizeof(tmpbuf));
+	return simple_read_from_buffer(buffer, *lenp, ppos, tmpbuf, len);
 }
 
 static int

-- 

WARNING: multiple messages have this Message-ID (diff)
From: Greg KH <gregkh@suse.de>
To: linux-kernel@vger.kernel.org, stable@kernel.org
Cc: Justin Forbes <jmforbes@linuxtx.org>,
	Zwane Mwaikambo <zwane@arm.linux.org.uk>,
	"Theodore Ts'o" <tytso@mit.edu>,
	Randy Dunlap <rdunlap@xenotime.net>,
	Dave Jones <davej@redhat.com>,
	Chuck Wolber <chuckw@quantumlinux.com>,
	Chris Wedgwood <reviews@ml.cw.f00f.org>,
	Michael Krufky <mkrufky@linuxtv.org>,
	Chuck Ebbert <cebbert@redhat.com>,
	Domenico Andreoli <cavokz@gmail.com>, Willy Tarreau <w@1wt.eu>,
	Rodrigo Rubira Branco <rbranco@la.checkpoint.com>,
	Jake Edge <jake@lwn.net>, Eugene Teo <eteo@redhat.com>,
	torvalds@linux-foundation.org, akpm@linux-foundation.org,
	alan@lxorguk.ukuu.org.uk, linux-nfs@vger.kernel.org,
	Greg Banks <gnb@sgi.com>, Neil Brown <neilb@suse.de>,
	"J. Bruce Fields" <bfields@citi.umich.edu>,
	Tom Tucker <tom@opengridcomputing.com>,
	Ingo Oeser <ioe-lkml@rameria.de>,
	Cyrill Gorcunov <gorcunov@gmail.com>,
	Chuck Lever <chuck.lever@oracle.com>
Subject: [patch 03/16] sunrpc: fix possible overrun on read of /proc/sys/sunrpc/transports
Date: Wed, 3 Sep 2008 10:32:47 -0700	[thread overview]
Message-ID: <20080903173247.GD10429@suse.de> (raw)
In-Reply-To: <20080903173218.GA10429@suse.de>

[-- Attachment #1: sunrpc-fix-possible-overrun-on-read-of-proc-sys-sunrpc-transports.patch --]
[-- Type: text/plain, Size: 2235 bytes --]


2.6.25-stable review patch.  If anyone has any objections, please let us know.

------------------
From: Cyrill Gorcunov <gorcunov@gmail.com>

commit 27df6f25ff218072e0e879a96beeb398a79cdbc8 upstream

Vegard Nossum reported
----------------------
> I noticed that something weird is going on with /proc/sys/sunrpc/transports.
> This file is generated in net/sunrpc/sysctl.c, function proc_do_xprt(). When
> I "cat" this file, I get the expected output:
>    $ cat /proc/sys/sunrpc/transports
>    tcp 1048576
>    udp 32768

> But I think that it does not check the length of the buffer supplied by
> userspace to read(). With my original program, I found that the stack was
> being overwritten by the characters above, even when the length given to
> read() was just 1.

David Wagner added (among other things) that copy_to_user could be
probably used here.

Ingo Oeser suggested to use simple_read_from_buffer() here.

The conclusion is that proc_do_xprt doesn't check for userside buffer
size indeed so fix this by using Ingo's suggestion.

Reported-by: Vegard Nossum <vegard.nossum@gmail.com>
Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
CC: Ingo Oeser <ioe-lkml@rameria.de>
Cc: Neil Brown <neilb@suse.de>
Cc: Chuck Lever <chuck.lever@oracle.com>
Cc: Greg Banks <gnb@sgi.com>
Cc: Tom Tucker <tom@opengridcomputing.com>
Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

---
 net/sunrpc/sysctl.c |   18 ++++--------------
 1 file changed, 4 insertions(+), 14 deletions(-)

--- a/net/sunrpc/sysctl.c
+++ b/net/sunrpc/sysctl.c
@@ -60,24 +60,14 @@ static int proc_do_xprt(ctl_table *table
 			void __user *buffer, size_t *lenp, loff_t *ppos)
 {
 	char tmpbuf[256];
-	int len;
+	size_t len;
+
 	if ((*ppos && !write) || !*lenp) {
 		*lenp = 0;
 		return 0;
 	}
-	if (write)
-		return -EINVAL;
-	else {
-		len = svc_print_xprts(tmpbuf, sizeof(tmpbuf));
-		if (!access_ok(VERIFY_WRITE, buffer, len))
-			return -EFAULT;
-
-		if (__copy_to_user(buffer, tmpbuf, len))
-			return -EFAULT;
-	}
-	*lenp -= len;
-	*ppos += len;
-	return 0;
+	len = svc_print_xprts(tmpbuf, sizeof(tmpbuf));
+	return simple_read_from_buffer(buffer, *lenp, ppos, tmpbuf, len);
 }
 
 static int

-- 

  parent reply	other threads:[~2008-09-03 17:49 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20080903172849.927077124@mini.kroah.org>
2008-09-03 17:32 ` [patch 00/16] 2.6.25-stable review Greg KH
2008-09-03 17:32   ` [patch 01/16] x86: work around MTRR mask setting Greg KH
2008-09-03 17:32   ` [patch 02/16] USB: cdc-acm: dont unlock acm->mutex on error path Greg KH
2008-09-03 17:32   ` Greg KH [this message]
2008-09-03 17:32     ` [patch 03/16] sunrpc: fix possible overrun on read of /proc/sys/sunrpc/transports Greg KH
2008-09-03 17:32   ` [patch 04/16] r8169: balance pci_map / pci_unmap pair Greg KH
2008-09-03 17:32   ` [patch 05/16] nfsd: fix buffer overrun decoding NFSv4 acl Greg KH
2008-09-03 17:32     ` Greg KH
2008-09-03 17:32   ` [patch 06/16] mm: make setup_zone_migrate_reserve() aware of overlapping nodes Greg KH
2008-09-03 17:32   ` [patch 07/16] forcedeth: fix checksum flag Greg KH
2008-09-03 17:32   ` [patch 08/16] fbdefio: add set_page_dirty handler to deferred IO FB Greg KH
2008-09-03 17:33   ` [patch 09/16] crypto: authenc - Avoid using clobbered request pointer Greg KH
2008-09-03 17:33   ` [patch 10/16] cramfs: fix named-pipe handling Greg KH
2008-09-03 17:33   ` [patch 11/16] cifs: fix O_APPEND on directio mounts Greg KH
2008-09-03 17:33   ` [patch 12/16] sctp: fix potential panics in the SCTP-AUTH API Greg KH
2008-09-03 17:33   ` [patch 13/16] sctp: add verification checks to SCTP_AUTH_KEY option Greg KH
2008-09-03 17:33   ` [patch 14/16] sctp: correct bounds check in sctp_setsockopt_auth_key Greg KH
2008-09-03 17:33   ` [patch 15/16] sctp: fix random memory dereference with SCTP_HMAC_IDENT option Greg KH
2008-09-03 17:33   ` [patch 16/16] sch_prio: Fix nla_parse_nested_compat() regression Greg KH

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080903173247.GD10429@suse.de \
    --to=gregkh@suse.de \
    --cc=akpm@linux-foundation.org \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=bfields@citi.umich.edu \
    --cc=cavokz@gmail.com \
    --cc=cebbert@redhat.com \
    --cc=chuck.lever@oracle.com \
    --cc=chuckw-lrwSPXAIdEw7YuNMryXyOw@public.gmane.org \
    --cc=davej@redhat.com \
    --cc=eteo@redhat.com \
    --cc=gnb@sgi.com \
    --cc=gorcunov@gmail.com \
    --cc=ioe-lkml-MFZNYGWX9eyELgA04lAiVw@public.gmane.org \
    --cc=jake-T1hC0tSOHrs@public.gmane.org \
    --cc=jmforbes-a5Mqy2EUIFVAfugRpC6u6w@public.gmane.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=mkrufky-dJidKbW2IEtAfugRpC6u6w@public.gmane.org \
    --cc=neilb@suse.de \
    --cc=rbranco-OG872enO0lhw4vQD1pnt9QC/G2K4zDHf@public.gmane.org \
    --cc=rdunlap@xenotime.net \
    --cc=reviews-vFB7bnJMxjixIvdkapsUdQ@public.gmane.org \
    --cc=stable@kernel.org \
    --cc=tom@opengridcomputing.com \
    --cc=torvalds@linux-foundation.org \
    --cc=tytso@mit.edu \
    --cc=w@1wt.eu \
    --cc=zwane@arm.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.