On Thu, Mar 10, 2022 at 05:54:21PM +0100, Theo Buehler wrote:
> On Thu, Mar 10, 2022 at 05:51:46PM +0100, Claudio Jeker wrote:
> > On Thu, Mar 10, 2022 at 05:33:28PM +0100, Martin Vahlensieck wrote:
> > > Hi
> > >
> > > This pulls up and adjusts the check if i exceeds the bounds of pfds.
> > > Before it was technically wrong, as i > NPFDS means that the last
> > > write (i == NPFDS) was already out of bounds.
> >
> > I see no reason to pull up the check but the if condition should indeed be
> > greater or equal. One could consider to change this into an assert() but I
> > think I stick with the errx().
>
> Agreed. ok for the diff that just changes the checks to >=
Actually I was wrong, the check needs to happen at the start of the loop
not at the end else it does not work if the list is exactly the number of
elements to fill NPFDS.
for (first element; if not end; next element) {
if (i >= NPFDS)
errx();
/* do work */
i++;
/* here condition would trigger on last element */
}
> >
> > > Martin
> > >
> > >
> > > Index: http.c
> > > ===================================================================
> > > RCS file: /cvs/src/usr.sbin/rpki-client/http.c,v
> > > retrieving revision 1.53
> > > diff -u -p -r1.53 http.c
> > > --- http.c 10 Feb 2022 11:10:40 -0000 1.53
> > > +++ http.c 10 Mar 2022 16:28:48 -0000
> > > @@ -1820,6 +1820,10 @@ proc_http(char *bind_addr, int fd)
> > > if (timeout == INFTIM || diff < timeout)
> > > timeout = diff;
> > > }
> > > +
> > > + if (i >= NPFDS)
> > > + errx(1, "too many connections");
> > > +
> > > if (conn->state == STATE_WRITE_DATA)
> > > pfds[i].fd = conn->req->outfd;
> > > else
> > > @@ -1828,8 +1832,6 @@ proc_http(char *bind_addr, int fd)
> > > pfds[i].events = conn->events;
> > > conn->pfd = &pfds[i];
> > > i++;
> > > - if (i > NPFDS)
> > > - errx(1, "too many connections");
> > > }
> > > LIST_FOREACH(conn, &idle, entry) {
> > > if (conn->idle_time <= now)
> > > @@ -1840,12 +1842,14 @@ proc_http(char *bind_addr, int fd)
> > > if (timeout == INFTIM || diff < timeout)
> > > timeout = diff;
> > > }
> > > +
> > > + if (i >= NPFDS)
> > > + errx(1, "too many connections");
> > > +
> > > pfds[i].fd = conn->fd;
> > > pfds[i].events = POLLIN;
> > > conn->pfd = &pfds[i];
> > > i++;
> > > - if (i > NPFDS)
> > > - errx(1, "too many connections");
> > > }
> > >
> > > if (poll(pfds, i, timeout) == -1) {
> > >
> >
> > --
> > :wq Claudio
> >
>
--
:wq Claudio
