Non-SFI NaCl: Disallow futex call without FUTEX_PRIVATE_FLAG

Non-SFI NaCl: Disallow futex call without FUTEX_PRIVATE_FLAG

So that untrusted code cannot communicate with other
processes with futex.

TEST=out/Release/nacl_loader_unittests
TEST=out/Release/browser_tests --gtest_filter='*NaCl*NonSfi*'
TEST=trybots
BUG=359285

[hamaji, 2014-04-19 01:17:29]

I'm not good at futex so I don't know if this filter is sufficient, but my guess
is that we might not want to restrict futex too much until we switch to newlib.
As futex is a very internal API, the way glibc uses this may change.

[Mark Seaborn, 2014-04-19 15:26:19]

This seems to fail, only in PPAPINaClPNaClNonSfiTest.Audio, with:

../../sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.cc:**CRASHING**:seccomp-bpf
failure in syscall 0240
[12182:12232:0419/010109:ERROR:nacl_process_host.cc(312)] NaCl process exited
with status 11 (0xb)
BrowserTestBase signal handler received SIGTERM. Backtrace:
#0 0x00000957f18a base::debug::StackTrace::StackTrace()
#1 0x00000cfbd464 content::(anonymous namespace)::DumpStackTraceSignalHandler()
#2 0x0000b7707400 ([vdso]+0x3ff)
#3 0x0000b7707424 ([vdso]+0x423)
#4 0x0000b6554460 __poll
#5 0x0000b74fda3b g_poll
#6 0x0000b74f006e \u003Cunknown>
#7 0x0000b74f01c1 g_main_context_iteration
#8 0x00000960098d base::MessagePumpGlib::Run()
#9 0x0000095b51c5 base::MessageLoop::RunHandler()
#10 0x0000095d22b2 base::RunLoop::Run()
#11 0x00000cff7414 content::RunThisRunLoop()
#12 0x00000cff748a content::RunMessageLoop()
#13 0x000009552ae2 JavascriptTestObserver::Run()
#14 0x000009564692 PPAPITestBase::RunTestURL()
#15 0x000009566ba8 PPAPITestBase::RunTestViaHTTP()
#16 0x000008c0e7de PPAPINaClPNaClNonSfiTest_Audio_Test::RunTestOnMainThread()

That's odd.  It looks like poll() is calling the futex syscall (syscall 240 on
x86-32).

[Mark Seaborn, 2014-04-19 15:35:35]

On 2014/04/19 15:26:19, Mark Seaborn wrote:
> This seems to fail, only in PPAPINaClPNaClNonSfiTest.Audio, with:
> 
>
../../sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.cc:**CRASHING**:seccomp-bpf
> failure in syscall 0240
> [12182:12232:0419/010109:ERROR:nacl_process_host.cc(312)] NaCl process exited
> with status 11 (0xb)
> BrowserTestBase signal handler received SIGTERM. Backtrace:
> #0 0x00000957f18a base::debug::StackTrace::StackTrace()
> #1 0x00000cfbd464 content::(anonymous
namespace)::DumpStackTraceSignalHandler()
> #2 0x0000b7707400 ([vdso]+0x3ff)
> #3 0x0000b7707424 ([vdso]+0x423)
> #4 0x0000b6554460 __poll
> #5 0x0000b74fda3b g_poll

> That's odd.  It looks like poll() is calling the futex syscall (syscall 240 on
> x86-32).

Oops, no, that backtrace is from the SIGTERM, not from the futex syscall.

[hamaji, 2014-04-19 19:15:03]

Sorry, maybe I tested this patch with a wrong branch...

pthread_join uses lll_wait_tid. My understanding is that this is to wait the
signal from kernel. I'm not sure if I read newlib's code correctly, but it looks
like it doesn't use futex and the thread manager takes care of freeing resource?
I think we can work-around this anyway. For now, I think we might want to allow
FUTEX_WAIT with a TODO comment.

[hamaji, 2014-04-20 00:06:08]

linux_asan is failing again. Interestingly, it seems all tests are failing even
without this change. They are failing for the another change, too:
https://codereview.chromium.org/244013002/

I couldn't reproduce this issue locally with

GYP_DEFINES='use_goma=1 asan=1 lsan=1 linux_use_tcmalloc=0 
component=static_library'

and

out/Release/components_unittests --brave-new-test-launcher
--test-launcher-bot-mode --test-launcher-batch-limit=1 --verbose
--test-launcher-print-test-stdio=always --gtest_print_time --no-sandbox
--test-launcher-summary-output=/tmp/tmpE4AWa3

Maybe time to stop running these tests with asan?

[Mark Seaborn, 2014-04-22 06:20:59]

On 19 April 2014 12:15, <hamaji@chromium.org> wrote:

> Sorry, maybe I tested this patch with a wrong branch...
>
> pthread_join uses lll_wait_tid. My understanding is that this is to wait
> the
> signal from kernel. I'm not sure if I read newlib's code correctly, but it
> looks
> like it doesn't use futex and the thread manager takes care of freeing
> resource?
> I think we can work-around this anyway. For now, I think we might want to
> allow
> FUTEX_WAIT with a TODO comment.


Ah, interesting, this aspect hadn't occurred to me when I suggested
restricting to private futexes.

If we allow non-private FUTEX_WAIT but require other operations to be
private, it seems initially that this would prevent communicating between
processes:  a process would be able to listen but not send.  However,
that's not the case.  A process would be able to send via clone()+exit()
(since thread exit signals on the TID address that was passed to clone()).

However, it looks like non-private futexes no longer work on addresses that
are mapped read-only, so restricting to private futexes might not be
necessary in current kernels.  Let me look into this more...

Cheers,
Mark

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[hamaji, 2014-04-22 18:38:58]

> If we allow non-private FUTEX_WAIT but require other operations to be
> private, it seems initially that this would prevent communicating between
> processes:  a process would be able to listen but not send.  However,
> that's not the case.  A process would be able to send via clone()+exit()
> (since thread exit signals on the TID address that was passed to clone()).

Ah, very interesting!

> However, it looks like non-private futexes no longer work on addresses that
> are mapped read-only, so restricting to private futexes might not be
> necessary in current kernels.  Let me look into this more...

Thanks for your check in advance. To be honest I don't know futex well. I mean,
I didn't read "futexes are tricky" yet.

[hamaji, 2014-04-30 04:50:24]

Let's close this for now. This should be tracked in
https://code.google.com/p/chromium/issues/detail?id=367649