Re: [Libevent-users] coverity and event_new/bufferevent_socket_new

[Vincent Bernat <bernat@xxxxxxxx>]

 â 25 novembre 2013 18:07 CET, Nick Mathewson <nickm@xxxxxxxxxxxxx>Â:

>> Using coverity with libevent triggers a resource leaks for socket
>> descriptors registered to event_new or bufferevent_socket_new. To my
>> understanding, coverity can be taught globally for such cases.
>>
>> Since I have seen some coverity-related bugfixes in the changelog, I
>> wonder if something has already been tried on this front?
>
> Can you say more about the warnings in question and why they occur?  I
> maintain at least one application that uses Libevent and that's
> scanned by Coverity, and I don't believe we've seen that kind of
> warning.
>
> I know that ntp also uses Libevent and Coverity Scan together, and so
> far as I know event_new and/or bufferevent_socket_new hasn't been an
> issue there. (I hope Harlan will let me know otherwise if I'm wrong.)
>
> (I'm not saying you're mistaken about the warnings -- only that AFAICT
> they don't seem to be an inevitable consequence of using event_new(),
> and that I need more info to know what the trouble could be.)

For `event_new()`, the problem may be when it is bundled inside another
function. Maybe coverity doesn't go too deep. For
`bufferevent_socket_new()`, here is the code excerpt:

#v+
   	1. open_fn: Returning handle opened by function "accept(int, __SOCKADDR_ARG, socklen_t * restrict)".
   	2. var_assign: Assigning: "s" = handle returned from "accept(fd, __SOCKADDR_ARG({ .__sockaddr__ = NULL}), NULL)".
   	3. Condition "(s = accept(fd, __SOCKADDR_ARG({ .__sockaddr__ = NULL}), NULL)) == -1", taking false branch
352        if ((s = accept(fd, NULL, NULL)) == -1) {
353                log_warn("event", "unable to accept connection from socket");
354                return;
355        }
356        client = calloc(1, sizeof(struct lldpd_one_client));
   	4. Condition "!client", taking false branch
357        if (!client) {
358                log_warnx("event", "unable to allocate memory for new client");
359                close(s);
360                goto accept_failed;
361        }
362        client->cfg = cfg;
   	5. noescape: Resource "s" is not freed or pointed-to in function "levent_make_socket_nonblocking(int)".[show details]
363        levent_make_socket_nonblocking(s);
364        TAILQ_INSERT_TAIL(&lldpd_clients, client, next);
   	6. Condition "(client->bev = bufferevent_socket_new(cfg->g_base, s, BEV_OPT_CLOSE_ON_FREE)) == NULL", taking false branch
365        if ((client->bev = bufferevent_socket_new(cfg->g_base, s,
366                    BEV_OPT_CLOSE_ON_FREE)) == NULL) {
367                log_warnx("event", "unable to allocate a new buffer event for new client");
368                close(s);
369                goto accept_failed;
370        }
371        bufferevent_setcb(client->bev,
372            levent_ctl_recv, NULL, levent_ctl_event,
373            client);
374        bufferevent_enable(client->bev, EV_READ | EV_WRITE);
375        log_debug("event", "new client accepted");
376        /* s has been saved by bufferevent_socket_new */
377        /* coverity[leaked_handle] */
   	
CID 1131427 (#1 of 1): Resource leak (RESOURCE_LEAK)
7. leaked_handle: Handle variable "s" going out of scope leaks the handle.
378        return;
379accept_failed:
380        levent_ctl_free_client(client);
#v-

Maybe it is focusing on `levent_make_socket_nonblocking()` instead of
`buffervent_socket_new()` or maybe this is because the latest one is
bundled into a condition.
-- 
printk(KERN_WARNING "Warning: defective CD-ROM (volume sequence
number). Enabling \"cruft\" mount option.\n");
	2.2.16 /usr/src/linux/fs/isofs/inode.c
***********************************************************************
To unsubscribe, send an e-mail to majordomo@xxxxxxxxxxxxx with
unsubscribe libevent-users    in the body.