Opened 8 weeks ago

Closed 4 weeks ago

#31024 closed defect (fixed)

Coverity: circpadding: always check circpad_machine_current_state()

Reported by: nickm Owned by:
Priority: Medium Milestone: Tor: 0.4.1.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: 041-should coverity dgoulet-merge
Cc: mikeperry, asn Actual Points:
Parent ID: Points:
Reviewer: nickm Sponsor:

Description

We usually check circpad_machine_current_state()'s return value, but in 2 places we don't. Coverity thinks that we're messing up in those cases.

This is CID 1447297 [circpad_estimate_circ_rtt_on_received] and 1447291 [circpad_is_token_removal_supported].

I suggest that we change our code to test the pointer in those cases. If we're sure that it can't be NULL, we can is an if (BUG(...)) check.

Child Tickets

Change History (4)

comment:1 Changed 7 weeks ago by asn

Status: newneeds_review

Please check the last commit of https://github.com/torproject/tor/pull/1160 which fixes both #31027 and #31024.

comment:2 Changed 7 weeks ago by asn

Reviewer: mikeperry

comment:3 Changed 4 weeks ago by nickm

Keywords: dgoulet-merge added
Reviewer: mikeperrynickm
Status: needs_reviewmerge_ready

Taking over review, since this looks trivially correct to me. Let's merge it!

comment:4 Changed 4 weeks ago by dgoulet

Resolution: fixed
Status: merge_readyclosed

The PR is against master not 041 so I can't merge into 041 without massive conflicts.

I cherry picked the 2 commits into an 041 branch (no conflicts!) and merged that forward: ticket31024_041_01.

(Oh an extra one line commit for our lovely practracker...)

Note: See TracTickets for help on using tickets.