Bug 142982 – [Seccomp Filters] Add helpers to get XDG base directory locations

• Component: WebKit2
• Status: RESOLVED FIXED

[Michael Catanzaro]

To implement a semi-robust syscall policy, we need to know the actual XDG base directories, instead of hardcoding them. This will help with that.

[Michael Catanzaro]

Created attachment 249268 [details]
[Linux] Seccomp Filters: Add helper functions for common directories

[Michael Catanzaro]

Created attachment 256704 [details]
[Linux] Seccomp Filters: Add helper functions for common directories

[Michael Catanzaro]

Comment on attachment 256704 [details]
[Linux] Seccomp Filters: Add helper functions for common directories

This one's next, but I'm pretty sure I should find a better home for these files than WebKit2/Shared/linux/SeccompFilters. Maybe WebCore/platform/FileSystemGLib.cpp?

[Michael Catanzaro]

The point of these functions, btw, is to use the GLib base directory functions without actually making SyscallPolicy.cpp depend on GLib.

[Michael Catanzaro]

(In reply to comment #4)
> The point of these functions, btw, is to use the GLib base directory
> functions without actually making SyscallPolicy.cpp depend on GLib.

Which maybe is not a worthwhile venture, since GTK, EFL, and WPE all use GLib.

[Zan Dobersek]

I think it's okay to abstract away the GLib functions, but I'm not sure where to put the files.

One option could be Source/WebCore/platform/seccompfilters, or a similarly named directory. The addition is specific to this feature, and the GLib-specific implementation could go into a UserDirectoriesGLib.cpp or similar, along with the UserDirectories.h header. ('BaseDirectory' sound odd to me.)

FileSystem.h is a more generic API used throughout the code, but the additions would for the moment only be used in seccomp filters and nowhere else, so half of the ports would end up with unused stub implementations, or the additions would be #if-guarded away (which I dislike).

Dunno.

[Michael Catanzaro]

(In reply to comment #6)
> I think it's okay to abstract away the GLib functions, but I'm not sure
> where to put the files.
> 
> One option could be Source/WebCore/platform/seccompfilters, or a similarly
> named directory.

I'd rather keep all the seccompfilters stuff in the WebKit2 layer.

So that suggests we should keep it exactly where I added it in this patch, except perhaps renaming the file:

> The addition is specific to this feature, and the
> GLib-specific implementation could go into a UserDirectoriesGLib.cpp or
> similar, along with the UserDirectories.h header. ('BaseDirectory' sound odd
> to me.)

UserDirectories is fine by me. I picked BaseDirectory because the functions are wrappers for the XDG Base Directory Specification [1], but I don't care what we call the file.

[1] http://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html

[Zan Dobersek]

XDGBaseDirectory.h could fit then as well. Wasn't aware of the spec.

[Zan Dobersek]

Comment on attachment 256704 [details]
[Linux] Seccomp Filters: Add helper functions for common directories

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

The remaining thing I'd recommend is to rename the implementation file into BaseDirectoryGLib.cpp (or whatever name we end up choosing), since the implementation depends on and is specific to GLib-using ports.

> Source/WebKit2/Shared/linux/SeccompFilters/BaseDirectory.cpp:32
> +#include <glib.h>
> +
> +#if ENABLE(SECCOMP_FILTERS)

glib.h should be included after the ENABLE() guard.

[Michael Catanzaro]

Created attachment 257270 [details]
Patch

[Michael Catanzaro]

Created attachment 257271 [details]
Patch

[Michael Catanzaro]

(In reply to comment #9)
> The remaining thing I'd recommend is to rename the implementation file into
> BaseDirectoryGLib.cpp (or whatever name we end up choosing), since the
> implementation depends on and is specific to GLib-using ports.

OK, done. I went with XDGBaseDirectoryGLib.cpp.

> glib.h should be included after the ENABLE() guard.

Fixed.

[Michael Catanzaro]

Committed r187159: <http://trac.webkit.org/changeset/187159>