[Webkit-unassigned] [Bug 191360] [GTK][WPE] Bubblewrap launcher should not depend on memfd

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Nov 8 00:14:01 PST 2018


https://bugs.webkit.org/show_bug.cgi?id=191360

--- Comment #3 from Carlos Garcia Campos <cgarcia at igalia.com> ---
Comment on attachment 354090
  --> https://bugs.webkit.org/attachment.cgi?id=354090
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=354090&action=review

>> Source/WebCore/ChangeLog:3
>> +        [GTK][WPE] Bubblewrap launcher should not depend on memfd
> 
> It's a little unclear to me why this is desirable given that kernel 3.17 is positively ancient now, but OK.

The code si simpler using SharedMemory, IMO and SharedMemory now uses memfd when available.

>> Source/WebCore/platform/glib/FileSystemGlib.cpp:42
>> +#if HAVE(LINUX_MEMFD_H)
> 
> Here's what really confuses me. A grep of the entire codebase shows no other uses of LINUX_MEMFD_H. And your patch doesn't appear to add it. So how does this check work? Is it always false?
> 
> It's not the ideal check, either. The original code is designed to use memfd even if memfd.h doesn't exist, as long as the kernel is new enough. I guess it's perfectly fine to change this, since we have fallback in place, but it's not clear to me that the change here was intentional.

LINUX_MEMFD_H was added in r237922. Since we have a fallback now, I think it's simpler to only use it if the header is present.

>> Source/WebCore/platform/glib/FileSystemGlib.cpp:43
>> +#include <linux/memfd.h>
> 
> Looks like this is unused? You only use sycall() directly and don't seem to use anything declared in this header.

Right

>> Source/WebCore/platform/glib/FileSystemGlib.cpp:477
>> +            fileDescriptor = syscall(__NR_memfd_create, name, 0);
> 
> Since you're inside an #if HAVE(LINUX_MEMFD_H) guard, you know linux/memfd.h is available, and can use it instead of syscall(). We were using syscall() to ensure this works when memfd.h is not available.

Can't we have newer glibc with old kernel? that's what ENOSYS is detecting, isn't it?

>> Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp:-94
>> -    }
> 
> Well we lost the fd sealing code. I'm not sure how important this is, or why it's there in the first place. Are other processes somehow able to modify the fd and mess with the args passed to bwap without the sealing? Patrick, can you comment on this?

I don't think we need it at all, seals only work with memfd and other processes don't assume the fd was created with memfd AFAIK. Ideally we could fix SharedMemoty::createHandle to use the protection parameter, but I don't know how to do it.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20181108/07db0d4a/attachment.html>


More information about the webkit-unassigned mailing list