Skip to content
GitLab
Projects Groups Snippets
  • /
  • Help
    • Help
    • Support
    • Community forum
    • Submit feedback
    • Contribute to GitLab
  • Sign in
  • Tor Tor
  • Project information
    • Project information
    • Activity
    • Labels
    • Members
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributors
    • Graph
    • Compare
  • Issues 328
    • Issues 328
    • List
    • Boards
    • Service Desk
    • Milestones
  • Merge requests 32
    • Merge requests 32
  • CI/CD
    • CI/CD
    • Pipelines
    • Jobs
    • Schedules
  • Deployments
    • Deployments
    • Environments
    • Releases
  • Monitor
    • Monitor
    • Incidents
  • Analytics
    • Analytics
    • Value stream
    • CI/CD
    • Repository
  • Activity
  • Graph
  • Create a new issue
  • Jobs
  • Commits
  • Issue Boards
Collapse sidebar
  • The Tor Project
  • Core
  • TorTor
  • Issues
  • #40442
Closed
Open
Issue created Aug 04, 2021 by Nick Mathewson@nickm🌻Owner

CID 1489776: Questionable sign extension in congestion control logic

Looks like we're missing signed and unsigned here unintentionally. Let's resolve this by adding explicit casts as necessary so that we don't have to reason too hard about C's cursed integer promotion logic.

 /src/core/or/congestion_control_common.c: 789 in congestion_control_update_circuit_bdp()
783           uint64_t delta = now_usec - timestamp_usec;
784     
785           /* The acked data is in sendme_cnt-1 chunks, because we are counting the
786            * data that is processed by the other endpoint *between* all of these
787            * sendmes. There's one less gap between the sendmes than the number
788            * of sendmes. */
>>>     CID 1489776:  Integer handling issues  (SIGN_EXTENSION)
>>>     Suspicious implicit sign extension: "cc->sendme_inc" with type "uint8_t" (8 bits, unsigned) is promoted in "(sendme_cnt - 1) * cc->sendme_inc" to type "int" (32 bits, signed), then sign-extended to type "unsigned long" (64 bits, unsigned).  If "(sendme_cnt - 1) * cc->sendme_inc" is greater than 0x7FFFFFFF, the upper bits of the result will all be 1.
789           uint64_t cells = (sendme_cnt-1)*cc->sendme_inc;
790     
791           /* The bandwidth estimate is cells/delta, which when multiplied
792            * by min RTT obtains the BDP. However, we multiply first to
793            * avoid precision issues with the RTT being close to delta in size. */
794           sendme_rate_bdp = cells*cc->min_rtt_usec/delta;

cc: @mikeperry @dgoulet

To upload designs, you'll need to enable LFS and have an admin enable hashed storage. More information
Assignee
Assign to
Time tracking