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
_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-bugs
To unsubscribe, send any mail to "[email protected]"
