From owner-cvs-sys  Tue Nov 14 20:53:27 1995
Return-Path: owner-cvs-sys
Received: (from root@localhost)
          by freefall.freebsd.org (8.6.12/8.6.6) id UAA10295
          for cvs-sys-outgoing; Tue, 14 Nov 1995 20:53:27 -0800
Received: from jhome.DIALix.COM (jhome.DIALix.COM [192.203.228.69])
          by freefall.freebsd.org (8.6.12/8.6.6) with ESMTP id UAA10239
          ; Tue, 14 Nov 1995 20:52:56 -0800
Received: (from peter@localhost) by jhome.DIALix.COM (8.6.12/8.6.9) id MAA04435; Wed, 15 Nov 1995 12:51:02 +0800
Date: Wed, 15 Nov 1995 12:51:02 +0800 (WST)
From: Peter Wemm <peter@jhome.DIALix.COM>
To: Bruce Evans <bde@zeta.org.au>
cc: phk@critter.tfs.com, CVS-commiters@freefall.freebsd.org,
        cvs-sys@freefall.freebsd.org
Subject: Re: cvs commit: src/sys/kern kern_sysctl.c
In-Reply-To: <199511150429.PAA17458@godzilla.zeta.org.au>
Message-ID: <Pine.BSF.3.91.951115124212.309I-100000@jhome.DIALix.COM>
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII
Sender: owner-cvs-sys@FreeBSD.ORG
Precedence: bulk

On Wed, 15 Nov 1995, Bruce Evans wrote:
> 
> I noticed a whole class of (old) sysctl bugs.  Consider e.g.,
> setdomainname().  The string is copied in directly over the
> old string.  If the copyin() faults, the old string is trashed.
> sysctl() returns EFAULT but the caller has no way of knowing
> if the old value is trashed.

Remember, root doesn't make mistaks. :-)

> To avoid this, all copyin()s should go to temporary storage.
> The bad malloc() method worked better here :-).

Hmm. Poul-Henning has used a useracc(new, newlen, B_READ) before going 
into the sysctl handlers.  I thought this was sufficient to be reasonably 
sure that accidents aren't going to happen?

> >The interface is badly designed, how about this one:
> 
> >	get some variable
> >		old buffer too small,
> >		new buffer correct.
> 
> >it should return ENOMEM because it cannot copyout, but should the
> >new value be installed ?
> 
> mpp and I fixed sysctl_string() to copyout as much as fits.
> 4.4lite2 is still broken here (it returns immediately).  We decided
> to install the new value in the ENOMEM case.  This is probably
> wrong.

Well.. As long as root doesn't make mistaks it's not relevant as it never 
happens. :-)  Seriously though, It is a very definate grey area. I think you 
are right in both counts..  It should copyout as much as fits on ENOMEM, 
but if it's going to return an error, it probably should not attempt the 
copyin.  Incidently, I think this is the new behavior of Poul-Henning's 
code (if copyout fails, copyin not attempted).

-Peter

> Bruce
>