Re: [PATCH] hugetlbfs: enforce F_SEAL_EXEC in setattr

[David Hildenbrand (Arm)]

On 6/29/26 20:02, Eva Kurchatova wrote:
> Commit 6fd735518263 ('mm/memfd: add F_SEAL_EXEC') added the F_SEAL_EXEC
> seal to memfd, with the corresponding enforcement implemented in

$ git show 6fd735518263
fatal: ambiguous argument '6fd735518263': unknown revision or path not in the
working tree.

Is it

6fd7353829ca ?

commit 6fd7353829cafc4067aad9eea0dc95da67e7df16
Author: Daniel Verkamp <dverkamp@xxxxxxxxxxxx>
Date:   Thu Dec 15 00:12:01 2022 +0000

    mm/memfd: add F_SEAL_EXEC



> shmem_setattr(): once F_SEAL_EXEC is set, fchmod() of any of the execute
> bits (S_IXUSR|S_IXGRP|S_IXOTH) is rejected with -EPERM.
> 
> The hugetlbfs equivalent in hugetlbfs_setattr() was missed, even though
> hugetlbfs memfd files carry the same 'info->seals' bitmap and the same
> F_SEAL_EXEC bit can be added through memfd_create(... MFD_HUGETLB ...)
> plus fcntl(F_ADD_SEALS, F_SEAL_EXEC). As a result, the seal is silently
> ignored on hugetlbfs memfds and fchmod() may turn execute bits back on
> after the seal has been applied.
> 
> Mirror the shmem check in hugetlbfs_setattr() so that ATTR_MODE changes
> that flip any execute bit on a sealed inode return -EPERM. This also
> fixes the memfd_test 'hugetlbfs' SEAL-EXEC subtest, which previously
> aborted on:
> 
>     fchmod(/memfd:kern_memfd_seal_exec (deleted), 00777) didn't fail as expected
> 
> Fixes: 6fd735518263 ("mm/memfd: add F_SEAL_EXEC")
> Signed-off-by: Eva Kurchatova <eva.kurchatova@xxxxxxxxxxxxx>
> ---
>  fs/hugetlbfs/inode.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
> index 216e1a0dd0b2..812bf8e21b91 100644
> --- a/fs/hugetlbfs/inode.c
> +++ b/fs/hugetlbfs/inode.c
> @@ -841,6 +841,11 @@ static int hugetlbfs_setattr(struct mnt_idmap *idmap,
>  	if (error)
>  		return error;
>  
> +	if ((info->seals & F_SEAL_EXEC) && (ia_valid & ATTR_MODE)) {
> +		if ((inode->i_mode ^ attr->ia_mode) & 0111)
> +			return -EPERM;
> +	}

This matches the mm/shmem.c change but could be written as

if ((info->seals & F_SEAL_EXEC) && (ia_valid & ATTR_MODE) &&
    (inode->i_mode ^ attr->ia_mode) & 0111))
	return -EPERM;

But even better would be factoring that out into a common helper that checks that?

In any case change itself looks like the right thing to do for me.

-- 
Cheers,

David