| msg209284 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) *  |
Date: 2014-01-26 07:35 |
| Christian Heimes reported (http://permalink.gmane.org/gmane.comp.python.devel/145253) Coverity issue: """ ** CID 1164423: Division or modulo by zero (DIVIDE_BY_ZERO) /Modules/audioop.c: 1375 in audioop_ratecv_impl() ________________________________________________________________________________________________________ *** CID 1164423: Division or modulo by zero (DIVIDE_BY_ZERO) /Modules/audioop.c: 1375 in audioop_ratecv_impl() 1369 without spurious overflow is the challenge; we can 1370 settle for a reasonable upper bound, though, in this 1371 case ceiling(len/inrate) * outrate. */ 1372 1373 /* compute ceiling(len/inrate) without overflow */ 1374 Py_ssize_t q = len > 0 ? 1 + (len - 1) / inrate : 0; >>> CID 1164423: Division or modulo by zero (DIVIDE_BY_ZERO) >>> In expression "9223372036854775807L / q", division by expression "q" which may be zero has undefined behavior. 1375 if (outrate > PY_SSIZE_T_MAX / q / bytes_per_frame) 1376 str = NULL; 1377 else 1378 str = PyBytes_FromStringAndSize(NULL, 1379 q * outrate * bytes_per_frame); 1380 } """ This is false positive. len should be non-negative and a case of 0 already checked just several lines before. Is Coverity aware asserts? Perhaps this patch will silence it. |
|
|
| msg209290 - (view) |
Author: Terry J. Reedy (terry.reedy) * |
Date: 2014-01-26 08:30 |
| Coverity appears concerned about the division by q. It apparently knows inrate != 0. I do not see any division by len. If q is cleared up, I presume it will check bytes_per_frame. |
|
|
| msg209368 - (view) |
Author: Christian Heimes (christian.heimes) * |
Date: 2014-01-26 23:45 |
| Coverity is concerned about the value of `q` when `len < 0`. The expression Py_ssize_t q = len > 0 ? 1 + (len - 1) / inrate : 0; returns a positive, non-null value for len > 0. Another check ensures that len != 0 a couple of lines earlier. In theory it is possible that len < 0. After all it's a signed integer type. Coverity tries very hard to guess the intention of code. Because there is a check for len > 0, Coverity thinks that the code has to handle len < 0. IMO a good fix should check len >= 0 very early and replace that line with Py_ssize_t q = 1 + (len - 1) / inrate; |
|
|
| msg209371 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2014-01-27 00:12 |
| New changeset bf52f2dbfdde by Christian Heimes in branch 'default': Issue #20394: Attempt to silence CID 1164423: Division or modulo by zero in audioop_ratecv_impl() http://hg.python.org/cpython/rev/bf52f2dbfdde |
|
|
| msg209405 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2014-01-27 07:00 |
| If Coverity is silenced, this patch should be backported to 2.7 and 3.3. |
|
|
| msg209411 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2014-01-27 08:07 |
| New changeset 0a406b6fe61f by Christian Heimes in branch 'default': I forgot to add a Misc/NEWS entry for issue #20394 http://hg.python.org/cpython/rev/0a406b6fe61f |
|
|
| msg209412 - (view) |
Author: Christian Heimes (christian.heimes) * |
Date: 2014-01-27 08:09 |
| The commit has silenced coverity's warning. |
|
|
| msg223249 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2014-07-16 18:42 |
| Christian, what about 2.7? |
|
|
| msg228517 - (view) |
Author: Mark Lawrence (BreamoreBoy) * |
Date: 2014-10-05 00:43 |
| Can we have a decision as to whether or not the patch should be back ported to 2.7. |
|
|