The following reply was made to PR kern/181127; it has been noted by GNATS.
From: Garrett Cooper <yaneurab...@gmail.com>
To: Bruce Evans <b...@optusnet.com.au>
Cc: freebsd-gnats-sub...@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 16:00:53 -0700
On Aug 8, 2013, at 4:35 AM, Bruce Evans wrote:
> On Thu, 8 Aug 2013, Garrett Cooper wrote:
>=20
>>> 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.
>=20
> 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:
>>=20
>> Before:
>>=20
>> $ 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])) =3D=3D 0 not met
>> setdomainname_limit: [0.004173s] Passed.
>> setdomainname_perm: [0.005297s] Passed.
>> [0.029872s]
>=20
> 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_se=
tdomainname.c , =
https://github.com/yaneurabeya/freebsd/blob/master/lib/libc/tests/gen/t_se=
thostname.c if you're curious).
...
>> @@ -314,11 +314,11 @@ sysctl_hostname(SYSCTL_HANDLER_ARGS)
>>=20
>> 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,
>=20
> 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?
Thanks...=
_______________________________________________
freebsd-bugs@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-bugs
To unsubscribe, send any mail to "freebsd-bugs-unsubscr...@freebsd.org"
