Date: Thu, 8 Aug 2013 11:40:03 GMT
From: Bruce Evans <brde@optusnet.com.au>
To: freebsd-bugs@FreeBSD.org
Subject: Re: kern/181127: [PATCH] set{domain, host}name doesn't permit NUL terminated strings that are MAXHOSTNAMELEN long
Message-ID: <201308081140.r78Be3eX063103@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
The following reply was made to PR kern/181127; it has been noted by GNATS.
From: Bruce Evans <brde@optusnet.com.au>
To: Garrett Cooper <yaneurabeya@gmail.com>
Cc: freebsd-gnats-submit@freebsd.org, freebsd-bugs@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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201308081140.r78Be3eX063103>