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

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