From owner-freebsd-hackers  Tue Feb 13 12:19:24 2001
Delivered-To: freebsd-hackers@freebsd.org
Received: from mail.gmx.net (pop.gmx.de [194.221.183.20])
	by hub.freebsd.org (Postfix) with SMTP id 737ED37B491
	for <freebsd-hackers@freebsd.org>; Tue, 13 Feb 2001 12:19:11 -0800 (PST)
Received: (qmail 10061 invoked by uid 0); 13 Feb 2001 20:19:09 -0000
Received: from p3e9d44d7.dip.t-dialin.net (HELO forge.local) (62.157.68.215)
  by mail.gmx.net (mail07) with SMTP; 13 Feb 2001 20:19:09 -0000
Received: from thomas by forge.local with local (Exim 3.20 #1)
	id 14Slug-0000Pp-00
	for <freebsd-hackers@freebsd.org>; Tue, 13 Feb 2001 21:19:02 +0100
Date: Tue, 13 Feb 2001 21:19:02 +0100
From: Thomas Moestl <tmoestl@gmx.net>
To: freebsd-hackers@freebsd.org
Subject: robustness fix for SYSCTL_OUT
Message-ID: <20010213211902.A873@crow.dom2ip.de>
Mail-Followup-To: Thomas Moestl <tmoestl@gmx.net>,
	freebsd-hackers@freebsd.org
Mime-Version: 1.0
Content-Type: multipart/mixed; boundary="huq684BweRXVnRxX"
Content-Disposition: inline
User-Agent: Mutt/1.2.5i
Sender: owner-freebsd-hackers@FreeBSD.ORG
Precedence: bulk
X-Loop: FreeBSD.ORG


--huq684BweRXVnRxX
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

Hi,

the following is from sys/kern/kern_sysctl.c:

static int
sysctl_old_kernel(struct sysctl_req *req, const void *p, size_t l)
{
	size_t i = 0;

	if (req->oldptr) {
		i = l;
		if (i > req->oldlen - req->oldidx)
			i = req->oldlen - req->oldidx;
		if (i > 0)
			bcopy(p, (char *)req->oldptr + req->oldidx, i);
	}
	req->oldidx += l;
	if (req->oldptr && i != l)
		return (ENOMEM);
	return (0);
}

oldidx and oldlen are both size_t (unsigned). If l happens to be larger
than (req->oldlen - req->oldidx), ENOMEM is returned correctly, but
req->oldidx is increased by the full length. If a buggy caller does
not react on the error and calls SYSCTL_OUT again (SYSCTL_OUT normally
causes sysctl_old_kernel() or sysctl_old_user, which has a similar bug,
to be called), oldidx will be greater than oldlen, and since both are
unsigned, the if test will fail, so we will bcopy outside of the buffer
and no longer return ENOMEM.

Not that this does not matter if SYSCTL_OUT is used correctly, but for
the sake of robustness, I think it should be fixed.
Currently, there is one place in the -CURRENT kernel (that I know of)
that actually gets hit by this bug. -STABLE seems fine.

I propose the attached fix. Could it please be reviewed and commited if
correct?

	- thomas



--huq684BweRXVnRxX
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="sysctl4.diff"

*** sys.3/kern/kern_sysctl.c	Tue Feb 13 16:15:52 2001
--- sys/kern/kern_sysctl.c	Tue Feb 13 20:06:37 2001
***************
*** 817,824 ****
  
  	if (req->oldptr) {
  		i = l;
! 		if (i > req->oldlen - req->oldidx)
! 			i = req->oldlen - req->oldidx;
  		if (i > 0)
  			bcopy(p, (char *)req->oldptr + req->oldidx, i);
  	}
--- 817,827 ----
  
  	if (req->oldptr) {
  		i = l;
! 		if (req->oldlen <= req->oldidx)
! 			i = 0;
! 		else
! 			if (i > req->oldlen - req->oldidx)
! 				i = req->oldlen - req->oldidx;
  		if (i > 0)
  			bcopy(p, (char *)req->oldptr + req->oldidx, i);
  	}
***************
*** 914,921 ****
  	}
  	if (req->oldptr) {
  		i = l;
! 		if (i > req->oldlen - req->oldidx)
! 			i = req->oldlen - req->oldidx;
  		if (i > 0)
  			error = copyout(p, (char *)req->oldptr + req->oldidx,
  					i);
--- 917,927 ----
  	}
  	if (req->oldptr) {
  		i = l;
! 		if (req->oldlen <= req->oldidx)
! 			i = 0;
! 		else
! 			if (i > req->oldlen - req->oldidx)
! 				i = req->oldlen - req->oldidx;
  		if (i > 0)
  			error = copyout(p, (char *)req->oldptr + req->oldidx,
  					i);

--huq684BweRXVnRxX--


To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-hackers" in the body of the message