kern/181127: [PATCH] set{domain, host}name doesn't permit NUL terminated strings that are MAXHOSTNAMELEN long
Bruce Evans
brde at optusnet.com.au
Fri Aug 9 00:40:01 UTC 2013
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: Bruce Evans <brde at optusnet.com.au>, 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: Fri, 9 Aug 2013 10:39:37 +1000 (EST)
On Thu, 8 Aug 2013, Garrett Cooper wrote:
> On Aug 8, 2013, at 4:35 AM, Bruce Evans wrote:
>
>> 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).
>
> So the fix is bogus. Ok, missed that MAXHOSTNAMELEN was '\0' inclusive.
>
>>> 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}.
>
> Yes. It's kind of odd why NetBSD passes here, but this should work on FreeBSD as well as they aren't doing anything going out-of-bounds in the testcases (see https://github.com/yaneurabeya/freebsd/blob/master/lib/libc/tests/gen/t_setdomainname.c , https://github.com/yaneurabeya/freebsd/blob/master/lib/libc/tests/gen/t_sethostname.c if you're curious).
It uses MAXHOSTNAMELEN + 1 in (only) 1 place, and then seems to check that
setdomainname() on a null name with "length" MAXHOSTNAMELEN + 1 fails.
It doesn't seem to test any strings of nearly length MAXHOSTNAMELEN.
> ...
>
>>> @@ -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.)
>
> So translation is: is there's a bug in the sysctl handler after jail support was added and there's no reasonable way to fix it without reverting things back to their sane forms?
No. I suspected a bug in the jail support, but couldn't see any. You will
have to check with name and string lengths of nearly MAXHOSTNAMELEN + 1 on
(or whatever the kernel buffer size is for plain SYSCTL_STRING()) to see if
the jail support gives any differences.
Bruce
More information about the freebsd-bugs
mailing list