Claudio Jeker wrote: > On Mon, Sep 07, 2015 at 03:49:14PM -0400, Michael McConville wrote: > > It seems pretty clear to me that what was here was wrong. A field of > > a global struct was pointed at local array. The program logic was a > > little wacky, but this is my best estimate of what was intended. > > Input? > > > > > > Index: ping6.c > > =================================================================== > > RCS file: /cvs/src/sbin/ping6/ping6.c,v > > retrieving revision 1.112 > > diff -u -p -r1.112 ping6.c > > --- ping6.c 1 Sep 2015 19:53:23 -0000 1.112 > > +++ ping6.c 7 Sep 2015 19:44:15 -0000 > > @@ -1056,7 +1056,10 @@ pinger(void) > > memset(&iov, 0, sizeof(iov)); > > iov[0].iov_base = (caddr_t)outpack; > > iov[0].iov_len = cc; > > - smsghdr.msg_iov = iov; > > + smsghdr.msg_iov = calloc(1, sizeof(struct iovec)); > > + if (smsghdr.msg_iov == NULL) > > + return(1); > > + *smsghdr.msg_iov = iov[0]; > > smsghdr.msg_iovlen = 1; > > > > i = sendmsg(s, &smsghdr, 0); > > > > The current code is perfectly fine. msg_iov is a pointer and it points > to the iov array. When calling sendmsg the kernel will know how to > handle this pointer. There is no need to calloc() memory for this.
Yes, but the data that it points to is an array on pinger()'s stack. When the current pinger() call returns, that data is gone and smsghdr.msg_iov points to garbage. Remember, smsghdr is global. That said, if we're sure that smsghdr.msg_iov is only accessed from within the current pinger() call, this isn't a functional bug. It could become one in the future, though, if someone adds something. On a separate note, I just noticed that my use of calloc() here is a little awkward. I was initially allocating space for two structs. It could be malloc() instead. Anyway, the more I look at this, the more I think it should just be left as is, despite being a little precarious.
