Attachment #257114 for bug #142980

View Details Raw Unified Return to Bug 142980

| Differences between



2015-07-20  Michael Catanzaro  <mcatanzaro@igalia.com>

        [Seccomp] Should be easier to debug blocked syscalls
        https://bugs.webkit.org/show_bug.cgi?id=142980

        These should be printed even when not running in debug mode. There is no
        value in hiding errors from release build users.

        Reviewed by NOBODY (OOPS!).

        * Shared/linux/SeccompFilters/SeccompBroker.cpp:
        (WebKit::SeccompBroker::runLoop): Don't close stderr et. al. in release builds.
        * Shared/linux/SeccompFilters/Syscall.cpp:
        (WebKit::write_uint): Added.
        (WebKit::reportUnexpectedSyscall): Added.
        (WebKit::Syscall::createFromContext): Call reportUnexpectedSyscall. Also, no need to crash
        here in release builds.
        * Shared/linux/SeccompFilters/SyscallPolicy.cpp:
        (WebKit::SyscallPolicy::hasPermissionForPath): Print a warning when access is denied.
        (WebKit::SyscallPolicy::permissionToString): Added.
        * Shared/linux/SeccompFilters/SyscallPolicy.h: Add permissionToString.

2015-07-19  Michael Catanzaro  <mcatanzaro@igalia.com>

        Unreviewed, placate -Wmismatched-tags after r187011




NO_RETURN void SeccompBroker::runLoop(int socket)
{
    // Close unnecessary inherited file descriptors.
    for (int i = STDERR_FILENO + 1; i < FD_SETSIZE; ++i) {
#else
    int i = 0;
#endif
    // Close all inherited file descriptors other
    // than the socket to the sandboxed process.
    for (; i < FD_SETSIZE; ++i)
        if (i != socket)
            close(i);
    }

    while (true) {
        char buffer[messageMaxSize];

Lines 1-5 a/Source/WebKit2/Shared/linux/SeccompFilters/Syscall.cpp_sec1
- a/Source/WebKit2/Shared/linux/SeccompFilters/Syscall.cpp -1 / +40 lines
1	/*	1	/*
2	* Copyright (C) 2013 Intel Corporation. All rights reserved.	2	* Copyright (C) 2013 Intel Corporation. All rights reserved.
		3	* Copyright (C) 2015 Igalia S.L.
3	*	4	*
4	* Redistribution and use in source and binary forms, with or without	5	* Redistribution and use in source and binary forms, with or without
5	* modification, are permitted provided that the following conditions	6	* modification, are permitted provided that the following conditions
Lines 32-41 a/Source/WebKit2/Shared/linux/SeccompFilters/Syscall.cpp_sec2
32	#include "OpenSyscall.h"	33	#include "OpenSyscall.h"
33	#include "SigactionSyscall.h"	34	#include "SigactionSyscall.h"
34	#include "SigprocmaskSyscall.h"	35	#include "SigprocmaskSyscall.h"
		36	#include <limits>
35	#include <seccomp.h>	37	#include <seccomp.h>
		38	#include <string.h>
		39	#include <unistd.h>
36		40
37	namespace WebKit {	41	namespace WebKit {
38		42
		43	// The redundant "constexpr const" is to placate Clang's -Wwritable-strings.
		44	static constexpr const char* const message = "Blocked unexpected syscall: ";
		45
		46	// Since "sprintf" is not signal-safe, reimplement %d here. Based on code from
		47	// http://outflux.net/teach-seccomp by Will Drewry and Kees Cook, released under
		48	// the Chromium BSD license.
		49	static void writeUnsignedInt(char* buf, unsigned val)
		50	{
		51	int width = 0;
		52	unsigned tens;
		53
		54	if (!val) {
		55	strcpy(buf, "0");
		56	return;
		57	}
		58	for (tens = val; tens; tens /= 10)
		59	++width;
		60	buf[width] = '\0';
		61	for (tens = val; tens; tens /= 10)
		62	buf[--width] = '0' + (tens % 10);
		63	}
		64
		65	static void reportUnexpectedSyscall(int syscall)
		66	{
		67	char buf[128];
		68	#if defined(__has_builtin) && __has_builtin(__builtin_strlen)
		69	static_assert(__builtin_strlen(message) + std::numeric_limits<int>::digits10 + 1 < sizeof(buf), "Buffer too small");
		70	#endif
		71	strcpy(buf, message);
		72	writeUnsignedInt(buf + strlen(buf), syscall);
		73	strcat(buf, "\n");
		74	write(STDERR_FILENO, buf, strlen(buf));
		75	}
		76
39	std::unique_ptr<Syscall> Syscall::createFromContext(ucontext_t* ucontext)	77	std::unique_ptr<Syscall> Syscall::createFromContext(ucontext_t* ucontext)
40	{	78	{
41	mcontext_t* mcontext = &ucontext->uc_mcontext;	79	mcontext_t* mcontext = &ucontext->uc_mcontext;
Lines 54-60 std::unique_ptr<Syscall> Syscall::createFromContext(ucontext_t* ucontext) a/Source/WebKit2/Shared/linux/SeccompFilters/Syscall.cpp_sec3
54	case __NR_rt_sigaction:	92	case __NR_rt_sigaction:
55	return SigactionSyscall::createFromContext(mcontext);	93	return SigactionSyscall::createFromContext(mcontext);
56	default:	94	default:
57	CRASH();	95	reportUnexpectedSyscall(mcontext->gregs[REG_SYSCALL]);
		96	ASSERT_NOT_REACHED();
58	}	97	}
59		98
60	return nullptr;	99	return nullptr;



    free(basePath);
    free(canonicalPath);

    if ((permission & policy->value) == permission)
        return true;

    // Don't warn if the file doesn't exist at all.
    if (!access(path, F_OK) || errno != ENOENT)
        fprintf(stderr, "Blocked impermissible %s access to %s\n", SyscallPolicy::permissionToString(permission), path);
    return false;
}

void SyscallPolicy::addFilePermission(const String& path, Permission permission)

#endif
}

const char* SyscallPolicy::permissionToString(Permission permission)
{
    switch (permission) {
    case Read:
        return "read";
    case Write:
        return "write";
    case ReadAndWrite:
        return "read/write";
    case NotAllowed:
        return "disallowed";
    }

    ASSERT_NOT_REACHED();
    return "";
}

} // namespace WebKit

#endif // ENABLE(SECCOMP_FILTERS)

Lines 52-57 public: a/Source/WebKit2/Shared/linux/SeccompFilters/SyscallPolicy.h_sec1
- a/Source/WebKit2/Shared/linux/SeccompFilters/SyscallPolicy.h +2 lines
52		52
53	void addDefaultWebProcessPolicy(const WebProcessCreationParameters&);	53	void addDefaultWebProcessPolicy(const WebProcessCreationParameters&);
54		54
		55	static const char* permissionToString(Permission);
		56
55	private:	57	private:
56	typedef HashMap<String, int> PermissionMap;	58	typedef HashMap<String, int> PermissionMap;
57	PermissionMap m_filePermission;	59	PermissionMap m_filePermission;

Return to Bug 142980

Lines 320-335 void SeccompBroker::initialize() a/Source/WebKit2/Shared/linux/SeccompFilters/SeccompBroker.cpp_sec1
- a/Source/WebKit2/Shared/linux/SeccompFilters/SeccompBroker.cpp -8 / +3 lines
320		320
321	NO_RETURN void SeccompBroker::runLoop(int socket)	321	NO_RETURN void SeccompBroker::runLoop(int socket)
322	{	322	{
323	#ifndef NDEBUG	323	// Close unnecessary inherited file descriptors.
324	int i = STDERR_FILENO + 1;	324	for (int i = STDERR_FILENO + 1; i < FD_SETSIZE; ++i) {
325	#else
326	int i = 0;
327	#endif
328	// Close all inherited file descriptors other
329	// than the socket to the sandboxed process.
330	for (; i < FD_SETSIZE; ++i)
331	if (i != socket)	325	if (i != socket)
332	close(i);	326	close(i);
		327	}
333		328
334	while (true) {	329	while (true) {
335	char buffer[messageMaxSize];	330	char buffer[messageMaxSize];

Lines 97-103 bool SyscallPolicy::hasPermissionForPath(const char* path, Permission permission a/Source/WebKit2/Shared/linux/SeccompFilters/SyscallPolicy.cpp_sec1
- a/Source/WebKit2/Shared/linux/SeccompFilters/SyscallPolicy.cpp -1 / +24 lines
97	free(basePath);	97	free(basePath);
98	free(canonicalPath);	98	free(canonicalPath);
99		99
100	return (permission & policy->value) == permission;	100	if ((permission & policy->value) == permission)
		101	return true;
		102
		103	// Don't warn if the file doesn't exist at all.
		104	if (!access(path, F_OK) \|\| errno != ENOENT)
		105	fprintf(stderr, "Blocked impermissible %s access to %s\n", SyscallPolicy::permissionToString(permission), path);
		106	return false;
101	}	107	}
102		108
103	void SyscallPolicy::addFilePermission(const String& path, Permission permission)	109	void SyscallPolicy::addFilePermission(const String& path, Permission permission)
Lines 256-261 void SyscallPolicy::addDefaultWebProcessPolicy(const WebProcessCreationParameter a/Source/WebKit2/Shared/linux/SeccompFilters/SyscallPolicy.cpp_sec2
256	#endif	262	#endif
257	}	263	}
258		264
		265	const char* SyscallPolicy::permissionToString(Permission permission)
		266	{
		267	switch (permission) {
		268	case Read:
		269	return "read";
		270	case Write:
		271	return "write";
		272	case ReadAndWrite:
		273	return "read/write";
		274	case NotAllowed:
		275	return "disallowed";
		276	}
		277
		278	ASSERT_NOT_REACHED();
		279	return "";
		280	}
		281
259	} // namespace WebKit	282	} // namespace WebKit
260		283
261	#endif // ENABLE(SECCOMP_FILTERS)	284	#endif // ENABLE(SECCOMP_FILTERS)

Lines 1-3 a/Source/WebKit2/ChangeLog_sec1
- a/Source/WebKit2/ChangeLog +22 lines
		1	2015-07-20 Michael Catanzaro <mcatanzaro@igalia.com>
		2
		3	[Seccomp] Should be easier to debug blocked syscalls
		4	https://bugs.webkit.org/show_bug.cgi?id=142980
		5
		6	These should be printed even when not running in debug mode. There is no
		7	value in hiding errors from release build users.
		8
		9	Reviewed by NOBODY (OOPS!).
		10
		11	* Shared/linux/SeccompFilters/SeccompBroker.cpp:
		12	(WebKit::SeccompBroker::runLoop): Don't close stderr et. al. in release builds.
		13	* Shared/linux/SeccompFilters/Syscall.cpp:
		14	(WebKit::write_uint): Added.
		15	(WebKit::reportUnexpectedSyscall): Added.
		16	(WebKit::Syscall::createFromContext): Call reportUnexpectedSyscall. Also, no need to crash
		17	here in release builds.
		18	* Shared/linux/SeccompFilters/SyscallPolicy.cpp:
		19	(WebKit::SyscallPolicy::hasPermissionForPath): Print a warning when access is denied.
		20	(WebKit::SyscallPolicy::permissionToString): Added.
		21	* Shared/linux/SeccompFilters/SyscallPolicy.h: Add permissionToString.
		22
1	2015-07-19 Michael Catanzaro <mcatanzaro@igalia.com>	23	2015-07-19 Michael Catanzaro <mcatanzaro@igalia.com>
2		24
3	Unreviewed, placate -Wmismatched-tags after r187011	25	Unreviewed, placate -Wmismatched-tags after r187011