On Wed, Aug 12, 2015 at 11:19:54AM -0500, Jamie Strandboge wrote: Disclaimer: I know nearly nothing about the apparmor insides, so please excuse any inappropriate ideas. Good catch, this makes sense, but to be strictly precise about this, I would say this makes the directory accessible for R/W, but readonly would be enough, wouldn't it? There could be a small code adjustment, I'd even dare calling it a clean-up, that would make it possible for this direcotry to be put in the 'restricted_rw'. It would change the semantic a bit, but since there is no path that could start with string from both 'restricted' and 'restricted_rw' currently, I don't see a problem there. Consider my idea of moving the line (and maybe some others from 'override') to 'restricted_rw' after applying the following patch. diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 4ce1e7a20590..2eae3eebdecf 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -595,18 +595,19 @@ valid_path(const char *path, const bool readonly) vah_warning(_("path does not exist, skipping file type checks")); opaths = sizeof(override)/sizeof(*(override)); - - npaths = sizeof(restricted)/sizeof(*(restricted)); - if (array_starts_with(path, restricted, npaths) == 0 && - array_starts_with(path, override, opaths) != 0) - return 1; + if (array_starts_with(path, override, opaths) == 0) + return 0; npaths = sizeof(restricted_rw)/sizeof(*(restricted_rw)); - if (!readonly) { + if (readonly) { if (array_starts_with(path, restricted_rw, npaths) == 0) - return 1; + return 0; } + npaths = sizeof(restricted)/sizeof(*(restricted)); + if (array_starts_with(path, restricted, npaths) != 0) + return 1; + return 0; } -- Martin