Hi, Kostantin, On 7/6/17 22:43, Konstantin Belousov wrote: >> 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.
Note that the previous code would just crash (due to NULL pointer deference) so I think this change is an improvement over the status quo. I do agree that the reliability of init(8) is critical and will see what we can do with the extreme situation and submit a new CR. Cheers,
signature.asc
Description: OpenPGP digital signature
