Opened 6 months ago
Closed 6 months ago
#30342 closed defect (fixed)
9 dephects on prob_distr.c (April 2019)
Reported by: | asn | Owned by: | |
---|---|---|---|
Priority: | Medium | Milestone: | Tor: 0.4.1.x-final |
Component: | Core Tor/Tor | Version: | |
Severity: | Normal | Keywords: | prob-distr coverity |
Cc: | catalyst, riastradh | Actual Points: | |
Parent ID: | Points: | 0.4 | |
Reviewer: | Sponsor: |
Description
Similar to #29805, coverity is again furious at prob distr:
We got a bunch of these:
_____________________________________________________________________________________________ *** CID 1444030: Incorrect expression (SIZEOF_MISMATCH) /src/test/test_prob_distr.c: 1254 in test_stochastic_weibull_impl() 1248 } 1249 1250 static bool 1251 test_stochastic_weibull_impl(double lambda, double k) 1252 { 1253 const struct weibull dist = { >>> CID 1444030: Incorrect expression (SIZEOF_MISMATCH) >>> Adding "0UL /* 0 * sizeof (&dist - (struct weibull const *)&dist) */" to pointer "&weibull_ops" of type "struct dist_ops const *" is >>> suspicious because adding an integral value to this pointer automatically scales that value by the size, 48 bytes, of the pointed-to type, >>> "struct dist_ops const". Most likely, the multiplication by "sizeof (0L)" in this expression is extraneous and should be eliminated. 1254 .base = WEIBULL(dist), 1255 .lambda = lambda, 1256 .k = k, 1257 }; 1258 1259 /*
and a bunch of these:
________________________________________________________________________________________________ *** CID 1415723: Incorrect expression (DIVIDE_BY_ZERO) /src/feature/client/circpathbias.c: 194 in pathbias_get_scale_ratio() 188 (void) options; 189 /** 190 * The mult factor is the numerator for our scaling 191 * of circuit counts for our path bias window. It 192 * allows us to scale by fractions. 193 */ >>> CID 1415723: Incorrect expression (DIVIDE_BY_ZERO) >>> In expression "networkstatus_get_param(NULL, "pb_multfactor", 1, 1, denominator) / (double)denominator", division by expression >>> "denominator" which may be zero has undefined behavior. 194 return networkstatus_get_param(NULL, "pb_multfactor", 195 1, 1, denominator)/((double)denominator); 196 } 197 198 /** The minimum number of circuit usage attempts before we start 199 * thinking about warning about path use bias and dropping guards */
Child Tickets
Change History (7)
comment:1 follow-up: 4 Changed 6 months ago by
comment:2 Changed 6 months ago by
Cc: | catalyst riastradh added |
---|
comment:3 follow-up: 5 Changed 6 months ago by
in src/lib/math/prob_distr.h:
53 * We define this conditionally to suppress false positives from 54 * Coverity, which gets confused by the sizeof business. 55 */ 56 #ifdef __COVERITY___ 57 #define TYPE_CHECK_OBJ(OPS, OBJ, TYPE) 0
There seems to be an extra underscore at the end of __COVERITY___
. I think all other occurrences end with two underscores, not three.
comment:4 Changed 6 months ago by
Replying to asn:
And
CID 1415723
has ator_assert(denominator > 0);
right above it....
I'm not seeing that tor_assert
either in scan.coverity.com or in the file on master.
comment:5 Changed 6 months ago by
Replying to catalyst:
in src/lib/math/prob_distr.h:
53 * We define this conditionally to suppress false positives from 54 * Coverity, which gets confused by the sizeof business. 55 */ 56 #ifdef __COVERITY___ 57 #define TYPE_CHECK_OBJ(OPS, OBJ, TYPE) 0There seems to be an extra underscore at the end of
__COVERITY___
. I think all other occurrences end with two underscores, not three.
Yes, you are right. Indeed, coverity seems to have a revision of the codebase with 3 underscores, but that's not the case in master (since 48a574604bef
): https://github.com/torproject/tor/blob/master/src/lib/math/prob_distr.h#L56
CID 1444641
was also fixed in #30180 and yet it still appears in the web interface.
Finally, wrt CID 1415723
(mentioned in the comment above), the CID does not appear in the web interface anymore (but i did receive it in an email on the 30th of April). The assert I was refering to is https://github.com/torproject/tor/blob/master/src/feature/client/circpathbias.c#L189
tl;dr coverity has a dated version of our codebase, and also sends emails about fixed issues. Not sure what we could do here, since I dont see a button that pulls the latest git rev.
comment:6 Changed 6 months ago by
I've tried manually kicking one off. (Our current coverity setup is a cronjob on my development machine.) I'm still having to tell it to ignore certificate verification, since curl still doesn't like scan.coverity.com.
comment:7 Changed 6 months ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
Oook. Just got 16 defect(s), reported by Coverity Scan earlier, were marked fixed in the recent build analyzed by Coverity Scan.
in the latest email, and all of these seem to be fixed. Thanks!
Created #30361 for two new ones.
Weird stuff all around.
The
SIZEOF_MISMATCH
errors are the ones we fixed in #29805.And
CID 1415723
has ator_assert(denominator > 0);
right above it....