Opened 4 weeks ago

Last modified 3 weeks ago

#24315 merge_ready defect

sandbox incompatible with glibc 2.26 (openat() not handled for all our files)

Reported by: dgoulet Owned by: nickm
Priority: Medium Milestone: Tor: 0.3.1.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: sandbox
Cc: Actual Points:
Parent ID: Points:
Reviewer: dgoulet Sponsor:

Description

If I enable the sandbox on my system, I get killed with:

(Sandbox) Caught a bad syscall attempt (syscall openat)
/usr/lib/x86_64-linux-gnu/libasan.so.4(+0x558c0)[0x7fb5203ab8c0]
/lib/x86_64-linux-gnu/libpthread.so.0(open64+0x4e)[0x7fb51ec6667e]
/lib/x86_64-linux-gnu/libpthread.so.0(+0x13150)[0x7fb51ec67150]
/lib/x86_64-linux-gnu/libpthread.so.0(open64+0x4e)[0x7fb51ec6667e]
/usr/lib/x86_64-linux-gnu/libevent-2.1.so.6(evutil_open_closeonexec_+0x20)[0x7fb51fbb5540]
/usr/lib/x86_64-linux-gnu/libevent-2.1.so.6(evutil_read_file_+0x53)[0x7fb51fbb5603]
/usr/lib/x86_64-linux-gnu/libevent-2.1.so.6(evdns_base_load_hosts+0x8b)[0x7fb51fbc429b]
/home/dgoulet/Documents/git/tor/src/or/tor(+0x9f7adf)[0x55aa44446adf]
/home/dgoulet/Documents/git/tor/src/or/tor(do_main_loop+0x745)[0x55aa440f01d5]
/home/dgoulet/Documents/git/tor/src/or/tor(tor_run_main+0x1895)[0x55aa440f4065]
/home/dgoulet/Documents/git/tor/src/or/tor(tor_main+0x86)[0x55aa440e1fb6]
/home/dgoulet/Documents/git/tor/src/or/tor(main+0x1c)[0x55aa440df20c]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf1)[0x7fb51db741c1]
/home/dgoulet/Documents/git/tor/src/or/tor(_start+0x2a)[0x55aa440e1c6a]

strace output:

19360 openat(AT_FDCWD, "/etc/hosts", O_RDONLY|O_CLOEXEC) = 257
19360 --- SIGSYS {si_signo=SIGSYS, si_code=SYS_SECCOMP, si_call_addr=0x7f7d90ab667e, si_syscall=__NR_openat, si_arch=AUDIT_ARCH_X86_64 ---}

It is the first file being opened _after_ the seccomp sandbox has been applied. Our sandbox code only considers "open()" to touch that file:

OPEN("/etc/hosts");

My libc is 2.26.

We probably need to handle the same files with openat() as we do with open() for this.

Child Tickets

Change History (10)

comment:1 Changed 4 weeks ago by nickm

So, in the libevent source code at that point, it calls open, not openat. Is libc turning that open() into an openat? Did the linux kernel unify the open and openat calls?

comment:2 Changed 4 weeks ago by dgoulet

Hmmm that glibc source code is kind of _crazy_ but I see that __libc_open() functions in sysdeps/unix/sysv/linux/open.c is calling:

  return SYSCALL_CANCEL (openat, AT_FDCWD, file, oflag, mode);

A small test only doing open() on a file does call openat() and nothing else.

So I assume they did (well at least on glibc 2.26 Ubuntu).

comment:3 Changed 4 weeks ago by nickm

the change seems to have been made with glibc commit b41152d716ee9c5ba34495a54e64ea2b732139b5, which first appeared in 2.26.

comment:4 Changed 4 weeks ago by nickm

we can detect the specific version at runtime with gnu_get_libc_version(). Of course, it would be cleaner to call open() and somehow discover whether open() or openat() is being called.

comment:5 Changed 4 weeks ago by dgoulet

Owner: changed from dgoulet to nickm
Reviewer: dgoulet

comment:6 Changed 4 weeks ago by nickm

Status: assignedneeds_review

ticket24315_029 has some necessary changes, I hope. I've tested a client a bit, but I haven't tested in too much detail.

One point for consideration during review -- I'm not sure whether the old openat() rules need to remain at all, or if there really were any "opens" that should have not have been openats.

comment:7 Changed 4 weeks ago by nickm

Summary: sandbox: openat() not handled for all our filessandbox incompatible with glibc 2.26 (openat() not handled for all our files)

comment:8 Changed 4 weeks ago by dgoulet

Status: needs_reviewmerge_ready

Branch lgtm; but I can't test this because IMm now getting:

(Sandbox) Caught a bad syscall attempt (syscall mprotect). I'll open a ticket for that.

comment:9 Changed 3 weeks ago by nickm

Milestone: Tor: 0.3.2.x-finalTor: 0.3.1.x-final

After review and testing from dgoulet, merging this to 0.3.2 and forward; marking for possible backport.

comment:10 Changed 3 weeks ago by Sebastian

cypherpunks notes that #24400 applies here, too.

Note: See TracTickets for help on using tickets.