Opened 3 years ago

Closed 3 years ago

#18901 closed defect (implemented)

Should we stop appling --enable-expensive-hardening to constant-time code ?

Reported by: nickm Owned by: nickm
Priority: Medium Milestone: Tor: 0.2.9.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: 029-backport, review-group-1
Cc: Actual Points:
Parent ID: #17983 Points: very-small
Reviewer: Sponsor:

Description

The ubsan and asan options introduce branch instructions all over the place. Although these branches are never actually taken in by code that doesn't immediately crash, I'm concerned that they might make our constant-time code less constant-time, with a suitably weird branch predictor.

(I have no evidence that this is actually happening, but the whole situation is a confusing mess.)

Out of an abundance of caution, I'd suggest that we make those options apply only to the non-constant-time code

Child Tickets

Change History (9)

comment:1 Changed 3 years ago by nickm

Status: newneeds_review

My ftrapv_v2 branch (for #17983) does this.

comment:2 Changed 3 years ago by nickm

Points: very-small

comment:3 Changed 3 years ago by nickm

Keywords: 029-nickm-says-yes added

marking these as the tickets I am in favor of for 029, among the 029-proposed ones.

comment:4 Changed 3 years ago by nickm

Keywords: 029-proposed 029-nickm-says-yes removed
Milestone: Tor: 0.2.9.x-final

Nobody objected to including any of these, so I guess they are in.

comment:5 Changed 3 years ago by nickm

Owner: set to nickm
Status: needs_reviewaccepted

comment:6 Changed 3 years ago by nickm

Parent ID: #17983
Status: acceptedneeds_review

comment:7 Changed 3 years ago by nickm

Keywords: review-group-1 added

comment:8 Changed 3 years ago by nickm

I've updated the branch containing this a little. It's now ftrapv_v3

comment:9 Changed 3 years ago by nickm

Resolution: implemented
Status: needs_reviewclosed

Merged parent.

Note: See TracTickets for help on using tickets.