Opened 3 years ago

Closed 3 years ago

#20063 closed defect (fixed)

Permit sched_yield in sandbox

Reported by: nickm Owned by: nickm
Priority: Medium Milestone: Tor: 0.2.9.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: review-group-8
Cc: Actual Points: 0
Parent ID: Points: 0
Reviewer: Sponsor:

Description


Child Tickets

Change History (6)

comment:1 Changed 3 years ago by nickm

Owner: set to nickm
Status: newaccepted

comment:2 Changed 3 years ago by nickm

Status: acceptedneeds_review

These are needed for --enable-expensive-hardening and sandbox 1 to work together.

See branch bug20063 in my public repository.

comment:3 Changed 3 years ago by nickm

Keywords: review-group-8 added

comment:4 Changed 3 years ago by asn

Hmm, is there a way to reproduce this problem, so that we can test the fix?

I tried running an unpatched tor (git-6abce601f22) with --enable-expensive-hardening and Sandbox 1 and encountered no problems with this torrc:

Sandbox 1
SocksPort auto

Tor bootstrapped to 100% no problem.

All in all, the patch seems like it's doing what is advertised, but I actually don't know anything about the workings of seccomp and our sandbox. A review by a more experienced person in this area would be appreciated.

One question: Should we only whitelist those syscalls if and only if ASAN is enabled? Because IIUC, now they are whitelisted for all builds.

comment:5 Changed 3 years ago by nickm

Hmm, is there a way to reproduce this problem, so that we can test the fix?

That's going to depend on your exact asan implementation; GCC and Clang are different a lot, and different versions of each will differ from one another. I think I ran into this problem with the asan from GCC 6.1.1.

Should we only whitelist those syscalls if and only if ASAN is enabled? Because IIUC, now they are whitelisted for all builds.

I think the calls are harmless, and we can just whitelist them generally, but I wouldn't mind if somebody had a look.

comment:6 Changed 3 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Merging this.

Note: See TracTickets for help on using tickets.