On Tue, 2020-09-08 at 08:52 -0400, Stephen Smalley wrote: > On Tue, Sep 8, 2020 at 8:50 AM Stephen Smalley > <stephen.smalley.work@xxxxxxxxx> wrote: > > > > On Tue, Sep 8, 2020 at 8:43 AM Mickaël Salaün <mic@xxxxxxxxxxx> wrote: > > > > > > > > > On 08/09/2020 14:28, Mimi Zohar wrote: > > > > Hi Mickael, > > > > > > > > On Tue, 2020-09-08 at 09:59 +0200, Mickaël Salaün wrote: > > > >> + mode |= MAY_INTERPRETED_EXEC; > > > >> + /* > > > >> + * For compatibility reasons, if the system-wide policy > > > >> + * doesn't enforce file permission checks, then > > > >> + * replaces the execute permission request with a read > > > >> + * permission request. > > > >> + */ > > > >> + mode &= ~MAY_EXEC; > > > >> + /* To be executed *by* user space, files must be readable. */ > > > >> + mode |= MAY_READ; > > > > > > > > After this change, I'm wondering if it makes sense to add a call to > > > > security_file_permission(). IMA doesn't currently define it, but > > > > could. > > > > > > Yes, that's the idea. We could replace the following inode_permission() > > > with file_permission(). I'm not sure how this will impact other LSMs though. I wasn't suggesting replacing the existing security_inode_permission hook later, but adding a new security_file_permission hook here. > > > > They are not equivalent at least as far as SELinux is concerned. > > security_file_permission() was only to be used to revalidate > > read/write permissions previously checked at file open to support > > policy changes and file or process label changes. We'd have to modify > > the SELinux hook if we wanted to have it check execute access even if > > nothing has changed since open time. > > Also Smack doesn't appear to implement file_permission at all, so it > would skip Smack checking. My question is whether adding a new security_file_permission call here would break either SELinux or Apparmor? Mimi