mheily / libkqueue

kqueue(2) compatibility library
Other
236 stars 77 forks source link

Coverity findings in 2.6.0 #129

Closed timwoj closed 2 years ago

timwoj commented 2 years ago

I recently updated the Zeek project to use libkqueue 2.6.0. Part of our CI pipeline is a pass through Coverity's Scan project. I normally disable checking third-party code but I apparently hadn't yet for libkqueue, and the 2.6.0 update had some new findings in it. They may all be false-positives, but I figured I should report them anyways. This is about the best I can do for reporting these as I'd have to add someone to the project on the Scan site to see the full details. Note all of these findings are on a Linux build.

Missing unlock in src/common/kqueue.c:

373     */
   5. Condition libkqueue_debug, taking true branch.
   6. lock: pthread_mutex_lock locks kq_mtx.mtx_lock.
   7. Condition libkqueue_debug, taking true branch.
374    tracing_mutex_lock(&kq_mtx);
   8. Condition (*kqops.kqueue_init)(kq) < 0, taking true branch.
375    if (kqops.kqueue_init(kq) < 0) {
376    error:
   9. Condition libkqueue_debug, taking true branch.
377        dbg_printf("kq=%p - init failed", kq);
378        tracing_mutex_destroy(&kq->kq_mtx);
379        free(kq);
380#ifndef _WIN32
381        pthread_setcancelstate(prev_cancel_state, NULL);
382#endif
   CID 1487877 (#1 of 1): Missing unlock (LOCK)10. missing_unlock: Returning without unlocking kq_mtx.mtx_lock.
383        return (-1);

Dead code in src/linux/user.c:

114    /* Create an eventfd */
115    evfd = eventfd(0, 0);
   cond_at_most: Condition evfd < 0, taking true branch. Now the value of evfd is at most -1.
116    if (evfd < 0) {
117        if ((errno == EMFILE) || (errno == ENFILE)) {
118            dbg_perror("eventfd(2) fd_used=%u fd_max=%u", get_fd_used(), get_fd_limit());
119        } else {
120            dbg_perror("eventfd(2)");
121        }
   at_most: At condition evfd >= 0, the value of evfd must be at most -1.
   dead_error_condition: The condition evfd >= 0 cannot be true.
   CID 1487876 (#1 of 1): Logically dead code (DEADCODE)dead_error_line: Execution cannot reach this statement: close(evfd);.
122        if (evfd >= 0) close(evfd);

Double unlock in src/posix/proc.c:

338                    waiter_notify_error(ppd, errno);
339
   12. unlock: pthread_mutex_unlock unlocks proc_pid_index_mtx.mtx_lock.
   13. Condition libkqueue_debug, taking true branch.
340                    tracing_mutex_unlock(&proc_pid_index_mtx);
341
   14. Continuing loop.
342                    continue;
343
344                case EINTR:
345                    goto again;
346                }
347            }
348
349            status = waiter_siginfo_to_status(&info);
350            if (status >= 0) waiter_notify(ppd, status);  /* If < 0 notification is spurious */
351        }
   CID 1487875 (#2 of 2): Double unlock (LOCK)16. double_unlock: pthread_mutex_unlock unlocks proc_pid_index_mtx.mtx_lock while it is unlocked.
352        tracing_mutex_unlock(&proc_pid_index_mtx);
arr2036 commented 2 years ago

Thanks. The dead code warning was mostly a none issue, but the other two were valid. I'll try and set Coverity up for libkqueue directly. There's probably quite a few other issues given that I'm not sure libkqueue has ever been run through a static analysis tool before.