On Fri, Aug 04, 2023 at 11:40:36AM +0200, Claudio Jeker wrote: > When copying the shutdown reason from ctl_neighbor into the peer struct > the strlcpy needs a NUL terminated string input. This may not be the case > so we should be more careful here. > I see two ways to fix this. > a) force in a NUL before callin strlcpy() as done below. > b) use memcpy() and then force terminate p->conf.reason.
I think either approach is fine. A third option would be c) snprintf with "%.*s" The downside is that it uses two sizeof (plus one in the error check) and there is the annoying size_t vs int issue of .*. > What is the preferred way? Whatever you like best. I'm ok with the below. > -- > :wq Claudio > > ? obj > Index: control.c > =================================================================== > RCS file: /cvs/src/usr.sbin/bgpd/control.c,v > retrieving revision 1.112 > diff -u -p -r1.112 control.c > --- control.c 4 Aug 2023 09:20:12 -0000 1.112 > +++ control.c 4 Aug 2023 09:35:37 -0000 > @@ -388,14 +388,20 @@ control_dispatch_msg(struct pollfd *pfd, > control_result(c, CTL_RES_OK); > break; > case IMSG_CTL_NEIGHBOR_DOWN: > - p->conf.down = 1; > + neighbor->reason[ > + sizeof(neighbor->reason) - 1] = > + '\0'; > strlcpy(p->conf.reason, > neighbor->reason, > sizeof(p->conf.reason)); > + p->conf.down = 1; > session_stop(p, ERR_CEASE_ADMIN_DOWN); > control_result(c, CTL_RES_OK); > break; > case IMSG_CTL_NEIGHBOR_CLEAR: > + neighbor->reason[ > + sizeof(neighbor->reason) - 1] = > + '\0'; > strlcpy(p->conf.reason, > neighbor->reason, > sizeof(p->conf.reason)); >
