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
*** 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);
