Re: LSM hook ordering in shmem_mknod() and shmem_tmpfile()?

[Hugh Dickins <hughd@google.com>]

On Wed, 30 Aug 2023, Paul Moore wrote:

> Hello all,
> 
> While looking at some recent changes in mm/shmem.c I noticed that the
> ordering between simple_acl_create() and
> security_inode_init_security() is different between shmem_mknod() and
> shmem_tmpfile().  In shmem_mknod() the ACL call comes before the LSM
> hook, and in shmem_tmpfile() the LSM call comes before the ACL call.
> 
> Perhaps this is correct, but it seemed a little odd to me so I wanted
> to check with all of you to make sure there is a good reason for the
> difference between the two functions.  Looking back to when
> shmem_tmpfile() was created ~2013 I don't see any explicit mention as
> to why the ordering is different so I'm looking for a bit of a sanity
> check to see if I'm missing something obvious.
> 
> My initial thinking this morning is that the
> security_inode_init_security() call should come before
> simple_acl_create() in both cases, but I'm open to different opinions
> on this.

Good eye.  The crucial commit here appears to be Mimi's 3.11 commit
37ec43cdc4c7 "evm: calculate HMAC after initializing posix acl on tmpfs"
which intentionally moved shmem_mknod()'s generic_acl_init() up before
the security_inode_init_security(), around the same time as Al was
copying shmem_mknod() to introduce shmem_tmpfile().

I'd have agreed with you, Paul, until reading Mimi's commit:
now it looks more like shmem_tmpfile() is the one to be changed,
except (I'm out of my depth) maybe it's irrelevant on tmpfiles.

Anyway, I think it's a question better answered by Mimi and Al.

Thanks,
Hugh