Bug 142982 - [Seccomp Filters] Add helpers to get XDG base directory locations
Summary: [Seccomp Filters] Add helpers to get XDG base directory locations
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Enhancement
Assignee: Michael Catanzaro
URL:
Keywords:
Depends on:
Blocks: 140072 142987
  Show dependency treegraph
 
Reported: 2015-03-23 14:02 PDT by Michael Catanzaro
Modified: 2015-07-22 08:58 PDT (History)
2 users (show)

See Also:


Attachments
[Linux] Seccomp Filters: Add helper functions for common directories (6.50 KB, patch)
2015-03-23 14:11 PDT, Michael Catanzaro
no flags Details | Formatted Diff | Diff
[Linux] Seccomp Filters: Add helper functions for common directories (6.50 KB, patch)
2015-07-13 08:42 PDT, Michael Catanzaro
no flags Details | Formatted Diff | Diff
Patch (6.49 KB, patch)
2015-07-22 08:14 PDT, Michael Catanzaro
no flags Details | Formatted Diff | Diff
Patch (6.49 KB, patch)
2015-07-22 08:18 PDT, Michael Catanzaro
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Catanzaro 2015-03-23 14:02:59 PDT
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.
Comment 1 Michael Catanzaro 2015-03-23 14:11:00 PDT
Created attachment 249268 [details]
[Linux] Seccomp Filters: Add helper functions for common directories
Comment 2 Michael Catanzaro 2015-07-13 08:42:00 PDT
Created attachment 256704 [details]
[Linux] Seccomp Filters: Add helper functions for common directories
Comment 3 Michael Catanzaro 2015-07-20 13:13:35 PDT
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?
Comment 4 Michael Catanzaro 2015-07-20 13:14:54 PDT
The point of these functions, btw, is to use the GLib base directory functions without actually making SyscallPolicy.cpp depend on GLib.
Comment 5 Michael Catanzaro 2015-07-20 13:16:25 PDT
(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.
Comment 6 Zan Dobersek 2015-07-22 06:05:19 PDT
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.
Comment 7 Michael Catanzaro 2015-07-22 06:55:12 PDT
(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
Comment 8 Zan Dobersek 2015-07-22 07:15:18 PDT
XDGBaseDirectory.h could fit then as well. Wasn't aware of the spec.
Comment 9 Zan Dobersek 2015-07-22 07:18:15 PDT
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.
Comment 10 Michael Catanzaro 2015-07-22 08:14:13 PDT
Created attachment 257270 [details]
Patch
Comment 11 Michael Catanzaro 2015-07-22 08:18:29 PDT
Created attachment 257271 [details]
Patch
Comment 12 Michael Catanzaro 2015-07-22 08:19:14 PDT
(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.
Comment 13 Michael Catanzaro 2015-07-22 08:58:13 PDT
Committed r187159: <http://trac.webkit.org/changeset/187159>