Hi,

I went through the warnings reported by the clang static analyzer (version 3.3) 
on the portable version (commit a6d666ecde6610b7093d6800d4c65fdb74644d0e), 
looking only at warnings in smtpd/, and they seem to be false positives.

smtpd/mta.c:1181:15 Use after free:
===================================

mta_route_disable(struct mta_route *route, int penalty, int reason)
[...]
        if (route->flags & ROUTE_DISABLED) {
                runq_cancel(runq_route, NULL, route);
                mta_route_unref(route); /* from last call to here */
        }
        route->flags |= reason & ROUTE_DISABLED;
        runq_schedule(runq_route, time(NULL) + delay, NULL, route);
        mta_route_ref(route);

Clang thinks that refcount in mta_route_unref would drop to zero and cause 
route to be freed.
But the only way to get to ROUTE_DISABLED should be through 
mta_route_disable(), so refcount should never drop to 0 here, assuming we 
received a route with refcount > 0 to begin with:
mta_route_disable(route [refcount > 0])
  mta_route_ref(route) [refcount++]
mta_route_disable(route [refcount > 0])
  mta_route_unref(route [refcount--])
  mta_route_ref(route [refcount++])


smtpd/table_passwd.c:110 Memory leak
====================================
while ((buf = fgetln(fp, &len))) {
 [...]
 buf = lbuf

Clang thinks that memory allocated by buf is lost, but in fact it is a static 
buffer from fgetln, so this is fine.

smtpd/backends/scheduler_ram.c: 790 Dereference of null pointer
================================================================
Clang things q is NULL if evp->type is none of D_MTA, D_MDA, D_BOUNCE. 
Those seem to be the only valid values, but why is the 'type' field in struct 
rq_envelope an 'int' and not an enum like in scheduler_info?

smtpd/smtpd.c:1382 Result of operation is garbage or undefined
===============================================================
Clang doesn't know that profiling has same value in line 1375 and 1380: either 
both ifs are taken, or neither.

smtpd/makemap.c:218 Uninitialized argument value
=================================================
Again clang doesn't know that strcmp(source, "-") != 0 always returns the same 
value.

The remaining warnings are about RB_GENERATE(),TAILQ_FIRST(), TAILQ_REMOVE().
For example clang thinks that TAILQ_FIRST() will return the same element even 
after TAILQ_REMOVE() was called, hence all the use-after-free warnings.

Best regards,
--Edwin

-- 
You received this mail because you are subscribed to [email protected]
To unsubscribe, send a mail to: [email protected]

Reply via email to