Skip site navigation (1)Skip section navigation (2) 
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>
next in thread | raw e-mail | index | archive | help 

--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

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20010213211902.A873>