On Tue, Mar 30, 2021 at 05:51:38PM +0200, Claudio Jeker wrote:
> On Tue, Mar 30, 2021 at 05:45:39PM +0200, Theo Buehler wrote:
> > On Tue, Mar 30, 2021 at 05:30:19PM +0200, Claudio Jeker wrote:
> > > Found the hard way. http_new() call http_free() if http_resolv() failes.
> > > http_free() call http_fail() in that case since the state is not
> > > STATE_DONE. In the main poll loop another http_fail() call is made. This
> > > results in bad bad things.
> >
> > Ugh. This is ok if you want to land it separately.
> >
> > There is also an issue with close(outfd): http_free() closes
> > conn->outfd, so you get an EBADF here on http_resolv() failure.
> >
> > http_new() should probably take ownership of outfd and close it on
> > http_parse_uri() failure.
> >
> > So I think the entire h == NULL case should go away.
> >
>
> Like this?
Exactly. ok
>
> --
> :wq Claudio
>
> Index: http.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/http.c,v
> retrieving revision 1.11
> diff -u -p -r1.11 http.c
> --- http.c 29 Mar 2021 15:37:04 -0000 1.11
> +++ http.c 30 Mar 2021 15:50:24 -0000
> @@ -402,6 +402,7 @@ http_new(size_t id, char *uri, char *mod
> if (http_parse_uri(uri, &host, &port, &path) == -1) {
> free(uri);
> free(modified_since);
> + close(outfd);
> return NULL;
> }
>
> @@ -1197,10 +1198,7 @@ proc_http(char *bind_addr, int fd)
> io_str_read(fd, &mod);
>
> h = http_new(id, uri, mod, outfd);
> - if (h == NULL) {
> - close(outfd);
> - http_fail(id);
> - } else
> + if (h != NULL) {
> for (i = 0; i < MAX_CONNECTIONS; i++) {
> if (http_conns[i] != NULL)
> continue;
> @@ -1209,6 +1207,7 @@ proc_http(char *bind_addr, int fd)
> http_conns[i] = NULL;
> break;
> }
> + }
> }
> if (pfds[MAX_CONNECTIONS].revents & POLLOUT) {
> switch (msgbuf_write(&msgq)) {
