kern/181127: [PATCH] set{domain, host}name doesn't permit NUL terminated strings that are MAXHOSTNAMELEN long

[Bruce Evans]

The following reply was made to PR kern/181127; it has been noted by GNATS.

From: Bruce Evans <brde at optusnet.com.au>
To: Garrett Cooper <yaneurabeya at gmail.com>
Cc: freebsd-gnats-submit at freebsd.org, freebsd-bugs at freebsd.org
Subject: Re: kern/181127: [PATCH] set{domain, host}name doesn't permit NUL
 terminated strings that are MAXHOSTNAMELEN long
Date: Thu, 8 Aug 2013 21:35:45 +1000 (EST)

 On Thu, 8 Aug 2013, Garrett Cooper wrote:
 
 >> Synopsis:       [PATCH] set{domain,host}name doesn't permit NUL terminated strings that are MAXHOSTNAMELEN long
 > ...
 >> Description:
 > The noted link/patch fixes POSIX and generic requirement compliance for set{domain,host}name per the manpages by accounting for the fact that the string
 > must be NUL terminated.
 
 The bugs seem to be mainly in the tests, so the proposed fix enlarges them.
 MAXHOSTNAMELEN is already 1 larger than the POSIX limit {HOST_NAME_MAX}
 (see the sysconf(3) sources).
 
 > Found with the NetBSD t_set{domain,host}name testcases:
 >
 > Before:
 >
 > $ pwd
 > /usr/tests/lib/libc/gen
 > $ sudo atf-run t_setdomainname | atf-report
 > t_setdomainname (1/1): 3 test cases
 >    setdomainname_basic: [0.019497s] Failed: /usr/src/lib/libc/tests/gen/t_setdomainname.c:66: setdomainname(domains[i],sizeof(domains[i])) == 0 not met
 >    setdomainname_limit: [0.004173s] Passed.
 >    setdomainname_perm: [0.005297s] Passed.
 > [0.029872s]
 
 I'm not sure what these do, but according to the Synopsis,
 set{domain,host}name correctly doesn't permit NUL terminated strings that
 are MAXHOSTNAMELEN long (not counting space for the NUL).  MAXHOSTNAMELEN
 counts space for the NUL and is 1 larger than {HOST_NAME_MAX}.
 
 > diff --git a/sys/kern/kern_mib.c b/sys/kern/kern_mib.c
 > index c84d4b2..384c14d 100644
 > --- a/sys/kern/kern_mib.c
 > +++ b/sys/kern/kern_mib.c
 > @@ -266,7 +266,7 @@ sysctl_hostname(SYSCTL_HANDLER_ARGS)
 > {
 > 	struct prison *pr, *cpr;
 > 	size_t pr_offset;
 > -	char tmpname[MAXHOSTNAMELEN];
 > +	char tmpname[MAXHOSTNAMELEN+1];
 > 	int descend, error, len;
 >
 > 	/*
 
 The patch also adds some style bugs (missing spaces around binary operator
 '+').
 
 > @@ -314,11 +314,11 @@ sysctl_hostname(SYSCTL_HANDLER_ARGS)
 >
 > SYSCTL_PROC(_kern, KERN_HOSTNAME, hostname,
 >     CTLTYPE_STRING | CTLFLAG_RW | CTLFLAG_PRISON | CTLFLAG_MPSAFE,
 > -    (void *)(offsetof(struct prison, pr_hostname)), MAXHOSTNAMELEN,
 > +    (void *)(offsetof(struct prison, pr_hostname)), MAXHOSTNAMELEN+1,
 >     sysctl_hostname, "A", "Hostname");
 > SYSCTL_PROC(_kern, KERN_NISDOMAINNAME, domainname,
 >     CTLTYPE_STRING | CTLFLAG_RW | CTLFLAG_PRISON | CTLFLAG_MPSAFE,
 > -    (void *)(offsetof(struct prison, pr_domainname)), MAXHOSTNAMELEN,
 > +    (void *)(offsetof(struct prison, pr_domainname)), MAXHOSTNAMELEN+1,
 >     sysctl_hostname, "A", "Name of the current YP/NIS domain");
 > SYSCTL_PROC(_kern, KERN_HOSTUUID, hostuuid,
 >     CTLTYPE_STRING | CTLFLAG_RW | CTLFLAG_PRISON | CTLFLAG_MPSAFE,
 
 The sysctls were originally simple SYSCTL_STRING()s and I think they
 worked then.  Now they are quite complicated, to  support jails, etc.,
 but they still use sysctl_handle_string() so I think they handle
 (non)strings and (non)termination the same.  Note that
 sysctl_handle_string() doesn't actually return strings unless the
 buffer is large enough to hold the NUL terminator.  It just truncates.
 This is reflected in the gethostname(3) API.  The name length for
 gethostname() must be 1 larger than {HOST_NAME_MAX} to ensure
 getting a string.  OTOH, the name length for sethostname(3) should
 not include space for the NUL, so it must not be larger than
 {HOST_NAME_MAX}.  If it is larger than {HOST_NAME_MAX}, then the
 syscall will just fail.  If it is larger than the string length
 (to include the NUL and possibly more) but not larger than
 {HOST_NAME_MAX}, then the syscall will succeed and the string will
 just be terminated more than once.  (It would be safer to write NULs
 from the end of the string until the end of the buffer in all cases.)
 
 Bruce