From owner-freebsd-bugs@FreeBSD.ORG Thu Aug 8 11:40:04 2013 Return-Path: Delivered-To: freebsd-bugs@smarthost.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTP id A39649B for ; Thu, 8 Aug 2013 11:40:04 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:1900:2254:206c::16:87]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 9067E2A90 for ; Thu, 8 Aug 2013 11:40:04 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.7/8.14.7) with ESMTP id r78Be3qR063104 for ; Thu, 8 Aug 2013 11:40:03 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.7/8.14.7/Submit) id r78Be3eX063103; Thu, 8 Aug 2013 11:40:03 GMT (envelope-from gnats) Date: Thu, 8 Aug 2013 11:40:03 GMT Message-Id: <201308081140.r78Be3eX063103@freefall.freebsd.org> To: freebsd-bugs@FreeBSD.org Cc: From: Bruce Evans Subject: Re: kern/181127: [PATCH] set{domain, host}name doesn't permit NUL terminated strings that are MAXHOSTNAMELEN long X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list Reply-To: Bruce Evans List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 08 Aug 2013 11:40:04 -0000 The following reply was made to PR kern/181127; it has been noted by GNATS. From: Bruce Evans To: Garrett Cooper 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