From owner-freebsd-commit  Wed Nov 15 01:25:04 1995
Return-Path: owner-commit
Received: (from root@localhost)
          by freefall.freebsd.org (8.6.12/8.6.6) id BAA10841
          for freebsd-commit-outgoing; Wed, 15 Nov 1995 01:25:04 -0800
Received: (from root@localhost)
          by freefall.freebsd.org (8.6.12/8.6.6) id BAA10814
          for cvs-all-outgoing; Wed, 15 Nov 1995 01:24:58 -0800
Received: (from root@localhost)
          by freefall.freebsd.org (8.6.12/8.6.6) id BAA10780
          for cvs-sys-outgoing; Wed, 15 Nov 1995 01:24:50 -0800
Received: from tfs.com (tfs.com [140.145.250.1])
          by freefall.freebsd.org (8.6.12/8.6.6) with SMTP id BAA10762
          ; Wed, 15 Nov 1995 01:24:32 -0800
Received: from critter.tfs.com by tfs.com (smail3.1.28.1) with SMTP
	id m0tFe4z-0003vlC; Wed, 15 Nov 95 01:24 PST
Received: from localhost (localhost [127.0.0.1]) by critter.tfs.com (8.6.11/8.6.9) with SMTP id KAA02595; Wed, 15 Nov 1995 10:24:27 +0100
X-Authentication-Warning: localhost.tfs.com: Host localhost didn't use HELO protocol
To: Bruce Evans <bde@zeta.org.au>
cc: peter@jhome.dialix.com, CVS-commiters@freefall.freebsd.org,
        cvs-sys@freefall.freebsd.org, phk@freefall.freebsd.org
Subject: Re: cvs commit: src/sys/kern kern_sysctl.c 
In-reply-to: Your message of "Wed, 15 Nov 1995 15:29:43 +1100."
             <199511150429.PAA17458@godzilla.zeta.org.au> 
Date: Wed, 15 Nov 1995 10:24:26 +0100
Message-ID: <2593.816427466@critter.tfs.com>
From: Poul-Henning Kamp <phk@critter.tfs.com>
Sender: owner-commit@FreeBSD.ORG
Precedence: bulk

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

Hmm, yeah, but how to tell when to use it.

I could complicate the policy a bit:

	copyout(->old)
	if (error)
		return (error)
	copyin(<-new)
	if (error && we_did_copyout)
		copyin(<-old)
	return (error);

But is it really worth it ?

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

I think so...

--
Poul-Henning Kamp           | phk@FreeBSD.ORG       FreeBSD Core-team.
http://www.freebsd.org/~phk | phk@login.dknet.dk    Private mailbox.
whois: [PHK]                | phk@ref.tfs.com       TRW Financial Systems, Inc.
Future will arrive by its own means, progress not so.