From owner-svn-src-all@freebsd.org Fri Jul 7 05:43:09 2017 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id EC894D9AE18; Fri, 7 Jul 2017 05:43:09 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 93D7F6878B; Fri, 7 Jul 2017 05:43:09 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from tom.home (kib@localhost [127.0.0.1]) by kib.kiev.ua (8.15.2/8.15.2) with ESMTPS id v675h4RC056306 (version=TLSv1.2 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Fri, 7 Jul 2017 08:43:04 +0300 (EEST) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.10.3 kib.kiev.ua v675h4RC056306 Received: (from kostik@localhost) by tom.home (8.15.2/8.15.2/Submit) id v675h4Br056305; Fri, 7 Jul 2017 08:43:04 +0300 (EEST) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Fri, 7 Jul 2017 08:43:04 +0300 From: Konstantin Belousov To: Xin LI Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r320761 - head/sbin/init Message-ID: <20170707054304.GN1935@kib.kiev.ua> References: <201707070248.v672mtJV048240@repo.freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <201707070248.v672mtJV048240@repo.freebsd.org> User-Agent: Mutt/1.8.3 (2017-05-23) X-Spam-Status: No, score=-2.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FREEMAIL_FROM,NML_ADSP_CUSTOM_MED autolearn=no autolearn_force=no version=3.4.1 X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on tom.home X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 07 Jul 2017 05:43:10 -0000 On Fri, Jul 07, 2017 at 02:48:55AM +0000, Xin LI wrote: > Author: delphij > Date: Fri Jul 7 02:48:55 2017 > New Revision: 320761 > URL: https://svnweb.freebsd.org/changeset/base/320761 > > Log: > - Use strlcat() instead of strncat(). > - Use asprintf() and handle allocation errors. > > Reviewed by: kevlo > MFC after: 2 weeks > Differential Revision: https://reviews.freebsd.org/D11486 > > Modified: > head/sbin/init/init.c > > Modified: head/sbin/init/init.c > ============================================================================== > --- head/sbin/init/init.c Fri Jul 7 00:34:51 2017 (r320760) > +++ head/sbin/init/init.c Fri Jul 7 02:48:55 2017 (r320761) > @@ -1271,8 +1271,8 @@ new_session(session_t *sprev, struct ttyent *typ) > > sp->se_flags |= SE_PRESENT; > > - sp->se_device = malloc(sizeof(_PATH_DEV) + strlen(typ->ty_name)); > - sprintf(sp->se_device, "%s%s", _PATH_DEV, typ->ty_name); > + if (asprintf(&sp->se_device, "%s%s", _PATH_DEV, typ->ty_name) < 0) > + err(1, "asprintf"); > IMO this is wrong. init(8) too important for the system operations, and panicing the machine due to error from attempt creating getty session is not worth it. Either session should be disabled, or retried after some time, or some other measures taken, but please do not kill init just due to a local error. I would even argue that using snprintf() there and ignoring truncation is much better than err(), not least because the problem probably can only practically appear due to a misconfiguration. > /* > * Attempt to open the device, if we get "device not configured" > @@ -1315,8 +1315,8 @@ setupargv(session_t *sp, struct ttyent *typ) > free(sp->se_getty_argv_space); > free(sp->se_getty_argv); > } > - sp->se_getty = malloc(strlen(typ->ty_getty) + strlen(typ->ty_name) + 2); > - sprintf(sp->se_getty, "%s %s", typ->ty_getty, typ->ty_name); > + if (asprintf(&sp->se_getty, "%s %s", typ->ty_getty, typ->ty_name) < 0) > + err(1, "asprintf"); Same there. > sp->se_getty_argv_space = strdup(sp->se_getty); > sp->se_getty_argv = construct_argv(sp->se_getty_argv_space); > if (sp->se_getty_argv == NULL) { > @@ -1429,7 +1429,7 @@ start_window_system(session_t *sp) > if (sp->se_type) { > /* Don't use malloc after fork */ > strcpy(term, "TERM="); > - strncat(term, sp->se_type, sizeof(term) - 6); > + strlcat(term, sp->se_type, sizeof(term)); > env[0] = term; > env[1] = 0; > } > @@ -1493,7 +1493,7 @@ start_getty(session_t *sp) > if (sp->se_type) { > /* Don't use malloc after fork */ > strcpy(term, "TERM="); > - strncat(term, sp->se_type, sizeof(term) - 6); > + strlcat(term, sp->se_type, sizeof(term)); > env[0] = term; > env[1] = 0; > } else