prop140: Caches generate diffs as appropriate

changed milestone to %Tor: 0.3.1.x-final in legacy/trac

Trac:
Keywords: N/A deleted, prop140 added

Trac:
Owner: N/A to nickm
Status: new to assigned

Trac:
Actualpoints: N/A to 4
Keywords: prop140 deleted, prop140 TorCoreTeam201704 added

The compression part of this is done in consdiffmgr_compress. Once I wire everything up to the rest of Tor, and test a little harder, I'll call this branch needs_review.

This is now ready for review. Please review the following separate branches:

consdiffmgr_compress
actually_compute_diffs

Trac:
Status: assigned to needs_review
Actualpoints: 4 to 5

Gitlab links to those branches:

Trac:
Cc: N/A to ahf

Overall this looks good. I've added a couple of comments, but nothing serious.

Trac:
Status: needs_review to needs_revision

Tried fixing most issues, though I think one of them might not be worth fixing. See gitlab discussion for more info

Trac:
Status: needs_revision to needs_review

This looks good. Let's get it in. I agree with 705b72b08612b1c2bc20e347e72e3cb2083367c8 not helping to make the code cleaner. Let's skip that.

Trac:
Status: needs_review to merge_ready

okay. I've resolved a merge conflict locally; than I tried running under Chutney to make sure it wouldn't explode.

It exploded. :) Currently trying to diagnose.

13:03 < nickm> to review: I only crash with clang. And only when fragile hardening is disabled. 13:05 < nickm> clang version 3.9.1 (tags/RELEASE_391/final) 13:06 < nickm> branch ticket21648

Thread 2 "tor" received signal SIGSYS, Bad system call.
[Switching to Thread 0x7ffff4895700 (LWP 11482)]
0x00007ffff5fdee97 in mprotect () from /lib64/libc.so.6
Missing separate debuginfos, use: dnf debuginfo-install keyutils-libs-1.5.9-8.fc24.x86_64 krb5-libs-1.14.4-7.fc25.x86_64 libcom_err-1.43.3-1.fc25.x86_64 libevent-2.0.22-1.fc25.x86_64 libgcc-6.3.1-1.fc25.x86_64 libscrypt-1.21-2.fc24.x86_64 libseccomp-2.3.2-1.fc25.x86_64 libselinux-2.5-13.fc25.x86_64 libzstd-1.1.3-1.fc25.x86_64 openssl-libs-1.0.2k-1.fc25.x86_64 pcre-8.40-7.fc25.x86_64 xz-libs-5.2.2-2.fc24.x86_64 zlib-1.2.8-10.fc24.x86_64
(gdb) bt
#0  0x00007ffff5fdee97 in mprotect () from /lib64/libc.so.6
#1  0x00007ffff5f606aa in sysmalloc () from /lib64/libc.so.6
#2  0x00007ffff5f6163a in _int_malloc () from /lib64/libc.so.6
#3  0x00007ffff5f62f14 in malloc () from /lib64/libc.so.6
#4  0x00007ffff6b30527 in ZSTD_initCStream_advanced () from /lib64/libzstd.so.1
#5  0x00007ffff6b31048 in ZSTD_initCStream_usingDict ()
   from /lib64/libzstd.so.1
#6  0x00005555556cfccb in tor_zstd_compress_new (compress=<optimized out>, 
    method=<optimized out>, level=<optimized out>)
    at src/common/compress_zstd.c:201
#7  0x00005555556cee43 in tor_compress_new (compress=1, method=ZSTD_METHOD, 
    compression_level=BEST_COMPRESSION) at src/common/compress.c:451
#8  0x00005555556ce59f in tor_compress_impl (compress=<optimized out>, 
    out=<optimized out>, out_len=<optimized out>, 
    in=0x7ffff5fdee97 <mprotect+7> "H=\001\360\377\377s\001\303H\213\r\301\337+", in_len=1818, method=<optimized out>, compression_level=BEST_COMPRESSION, 
    complete_only=<optimized out>, protocol_warn_level=<optimized out>)
    at src/common/compress.c:98
#9  0x00005555556ce53e in tor_compress (out=0x1a0000, out_len=0x3, 
    in=0x7ffff5fdee97 <mprotect+7> "H=\001\360\377\377s\001\303H\213\r\301\337+", in_len=140737018593280, method=487424) at src/common/compress.c:213
#10 0x0000555555660053 in consensus_diff_worker_threadfn (
    state_=<optimized out>, work_=0x555555af97f0) at src/or/consdiffmgr.c:1225

no, false alarm there. My "disable threading" logic was itself b0rken.

I think I figured it out.

This happens when we go to allocate something fairly big in a worker thread, AND the linux seccomp2 sandbox is turned on, AND we're using the standard glibc allocator and not some other thing that took over for the glibc allocator. We never ran into this before, since the current workers don't allocate much.

Right now we have a 1-MB limit on how much RAM we allow the seccomp2 sandbox to mprotect(PROT_READ|PRO_WRITE) at once. I guess that that's not good enough for the threads, and it's trying to mprotect() more.

To confirm, I'll dump the syscall arguments for the above backtrace:

Thread 2 "tor" received signal SIGSYS, Bad system call.
[Switching to Thread 0x7ffff447c700 (LWP 2851)]
0x00007ffff5fdee97 in mprotect () from /lib64/libc.so.6
Missing separate debuginfos, use: dnf debuginfo-install keyutils-libs-1.5.9-8.fc24.x86_64 krb5-libs-1.14.4-7.fc25.x86_64 libcom_err-1.43.3-1.fc25.x86_64 libevent-2.0.22-1.fc25.x86_64 libgcc-6.3.1-1.fc25.x86_64 libscrypt-1.21-2.fc24.x86_64 libseccomp-2.3.2-1.fc25.x86_64 libselinux-2.5-13.fc25.x86_64 libzstd-1.1.3-1.fc25.x86_64 openssl-libs-1.0.2k-1.fc25.x86_64 pcre-8.40-7.fc25.x86_64 xz-libs-5.2.2-2.fc24.x86_64 zlib-1.2.8-10.fc24.x86_64
(gdb) print $rdi
$1 = 140737155325952
(gdb) print $rsi
$2 = 8880128
(gdb) print $rdx
$3 = 3

That's mprotect(0x7fffec266000, 880128, PROT_READ|PROT_WRITE.

Short-term solution -- raise MALLOC_MP_LIM.

Longer-term solution -- make the seccomp2 sandbox wiser about interned strings. We'll want to do that anyway here.

okay, that did it: legacy/trac#22096 (moved)

Merging at last.

Trac:
Resolution: N/A to implemented
Status: merge_ready to closed

closed

changed time estimate to 16h

added 40h of time spent

moved from legacy/trac#21648 (moved)

added Bug label and removed 1 deleted label

removed 1 deleted label

prop140: Caches generate diffs as appropriate

Child items ...

Activity