[PATCH] memfd: deny writeable mappings when implying SEAL_WRITE

[PATCH] memfd: deny writeable mappings when implying SEAL_WRITE

[Pratyush Yadav]

From: "Pratyush Yadav (Google)" <pratyush@kernel.org>

When SEAL_EXEC is added, SEAL_WRITE is implied to make W^X. But the
implied seal is set after the check that makes sure the memfd can not
have any writable mappings. This means one can use SEAL_EXEC to apply
SEAL_WRITE while having writeable mappings.

This breaks the contract that SEAL_WRITE provides and can be used by an
attacker to pass a memfd that appears to be write sealed but can still
be modified arbitrarily.

Fix this by adding the implied seals before the call for
mapping_deny_writable() is done.

Fixes: c4f75bc8bd6b ("mm/memfd: add write seals when apply SEAL_EXEC to executable memfd")
Cc: stable@vger.kernel.org
Signed-off-by: Pratyush Yadav (Google) <pratyush@kernel.org>
---
 mm/memfd.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/mm/memfd.c b/mm/memfd.c
index fb425f4e315f..abe13b291ddc 100644
--- a/mm/memfd.c
+++ b/mm/memfd.c
@@ -283,6 +283,12 @@ int memfd_add_seals(struct file *file, unsigned int seals)
 		goto unlock;
 	}
 
+	/*
+	 * SEAL_EXEC implies SEAL_WRITE, making W^X from the start.
+	 */
+	if (seals & F_SEAL_EXEC && inode->i_mode & 0111)
+		seals |= F_SEAL_SHRINK|F_SEAL_GROW|F_SEAL_WRITE|F_SEAL_FUTURE_WRITE;
+
 	if ((seals & F_SEAL_WRITE) && !(*file_seals & F_SEAL_WRITE)) {
 		error = mapping_deny_writable(file->f_mapping);
 		if (error)
@@ -295,12 +301,6 @@ int memfd_add_seals(struct file *file, unsigned int seals)
 		}
 	}
 
-	/*
-	 * SEAL_EXEC implies SEAL_WRITE, making W^X from the start.
-	 */
-	if (seals & F_SEAL_EXEC && inode->i_mode & 0111)
-		seals |= F_SEAL_SHRINK|F_SEAL_GROW|F_SEAL_WRITE|F_SEAL_FUTURE_WRITE;
-
 	*file_seals |= seals;
 	error = 0;
 
-- 
2.54.0.545.g6539524ca2-goog

Re: [PATCH] memfd: deny writeable mappings when implying SEAL_WRITE

[Pasha Tatashin]

On 05-05 15:39, Pratyush Yadav wrote:
> From: "Pratyush Yadav (Google)" <pratyush@kernel.org>
> 
> When SEAL_EXEC is added, SEAL_WRITE is implied to make W^X. But the
> implied seal is set after the check that makes sure the memfd can not
> have any writable mappings. This means one can use SEAL_EXEC to apply
> SEAL_WRITE while having writeable mappings.
> 
> This breaks the contract that SEAL_WRITE provides and can be used by an
> attacker to pass a memfd that appears to be write sealed but can still
> be modified arbitrarily.
> 
> Fix this by adding the implied seals before the call for
> mapping_deny_writable() is done.
> 
> Fixes: c4f75bc8bd6b ("mm/memfd: add write seals when apply SEAL_EXEC to executable memfd")
> Cc: stable@vger.kernel.org
> Signed-off-by: Pratyush Yadav (Google) <pratyush@kernel.org>

Reviewed-by: Pasha Tatashin <pasha.tatashin@soleen.com> 

> ---
>  mm/memfd.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/mm/memfd.c b/mm/memfd.c
> index fb425f4e315f..abe13b291ddc 100644
> --- a/mm/memfd.c
> +++ b/mm/memfd.c
> @@ -283,6 +283,12 @@ int memfd_add_seals(struct file *file, unsigned int seals)
>  		goto unlock;
>  	}
>  
> +	/*
> +	 * SEAL_EXEC implies SEAL_WRITE, making W^X from the start.
> +	 */
> +	if (seals & F_SEAL_EXEC && inode->i_mode & 0111)
> +		seals |= F_SEAL_SHRINK|F_SEAL_GROW|F_SEAL_WRITE|F_SEAL_FUTURE_WRITE;
> +
>  	if ((seals & F_SEAL_WRITE) && !(*file_seals & F_SEAL_WRITE)) {
>  		error = mapping_deny_writable(file->f_mapping);
>  		if (error)
> @@ -295,12 +301,6 @@ int memfd_add_seals(struct file *file, unsigned int seals)
>  		}
>  	}
>  
> -	/*
> -	 * SEAL_EXEC implies SEAL_WRITE, making W^X from the start.
> -	 */
> -	if (seals & F_SEAL_EXEC && inode->i_mode & 0111)
> -		seals |= F_SEAL_SHRINK|F_SEAL_GROW|F_SEAL_WRITE|F_SEAL_FUTURE_WRITE;
> -
>  	*file_seals |= seals;
>  	error = 0;
>  
> -- 
> 2.54.0.545.g6539524ca2-goog
> 

Re: [PATCH] memfd: deny writeable mappings when implying SEAL_WRITE

[Pasha Tatashin]

On Tue, 05 May 2026 15:39:20 +0200, Pratyush Yadav wrote:
> When SEAL_EXEC is added, SEAL_WRITE is implied to make W^X. But the
> implied seal is set after the check that makes sure the memfd can not
> have any writable mappings. This means one can use SEAL_EXEC to apply
> SEAL_WRITE while having writeable mappings.
> 
> This breaks the contract that SEAL_WRITE provides and can be used by an
> attacker to pass a memfd that appears to be write sealed but can still
> be modified arbitrarily.
> 
> [...]

Applied, thanks!

[1/1] memfd: deny writeable mappings when implying SEAL_WRITE
      commit: 73f496662a9848021e75742a69a3239ea850c3ee

Best regards,
-- 
Pasha Tatashin <pasha.tatashin@soleen.com>

Re: [PATCH] memfd: deny writeable mappings when implying SEAL_WRITE

[Pasha Tatashin]

On 05-05 15:37, Pasha Tatashin wrote:
> 
> On Tue, 05 May 2026 15:39:20 +0200, Pratyush Yadav wrote:
> > When SEAL_EXEC is added, SEAL_WRITE is implied to make W^X. But the
> > implied seal is set after the check that makes sure the memfd can not
> > have any writable mappings. This means one can use SEAL_EXEC to apply
> > SEAL_WRITE while having writeable mappings.
> > 
> > This breaks the contract that SEAL_WRITE provides and can be used by an
> > attacker to pass a memfd that appears to be write sealed but can still
> > be modified arbitrarily.
> > 
> > [...]
> 
> Applied, thanks!
> 
> [1/1] memfd: deny writeable mappings when implying SEAL_WRITE
>       commit: 73f496662a9848021e75742a69a3239ea850c3ee

^^^
Please ignore, this should be Applied to MM tree.

Pasha

> 
> Best regards,
> -- 
> Pasha Tatashin <pasha.tatashin@soleen.com>
> 

Re: [PATCH] memfd: deny writeable mappings when implying SEAL_WRITE

[Jeff Xu]

Hi Pratyush,

Thank you for fixing this.

On Tue, May 5, 2026 at 6:39 AM Pratyush Yadav <pratyush@kernel.org> wrote:
>
> From: "Pratyush Yadav (Google)" <pratyush@kernel.org>
>
> When SEAL_EXEC is added, SEAL_WRITE is implied to make W^X. But the
> implied seal is set after the check that makes sure the memfd can not
> have any writable mappings. This means one can use SEAL_EXEC to apply
> SEAL_WRITE while having writeable mappings.
>
> This breaks the contract that SEAL_WRITE provides and can be used by an
> attacker to pass a memfd that appears to be write sealed but can still
> be modified arbitrarily.
>
> Fix this by adding the implied seals before the call for
> mapping_deny_writable() is done.
>
> Fixes: c4f75bc8bd6b ("mm/memfd: add write seals when apply SEAL_EXEC to executable memfd")
> Cc: stable@vger.kernel.org
> Signed-off-by: Pratyush Yadav (Google) <pratyush@kernel.org>
Acked-by: Jeff Xu <jeffxu@google.com>

-Jeff

> ---
>  mm/memfd.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/mm/memfd.c b/mm/memfd.c
> index fb425f4e315f..abe13b291ddc 100644
> --- a/mm/memfd.c
> +++ b/mm/memfd.c
> @@ -283,6 +283,12 @@ int memfd_add_seals(struct file *file, unsigned int seals)
>                 goto unlock;
>         }
>
> +       /*
> +        * SEAL_EXEC implies SEAL_WRITE, making W^X from the start.
> +        */
> +       if (seals & F_SEAL_EXEC && inode->i_mode & 0111)
> +               seals |= F_SEAL_SHRINK|F_SEAL_GROW|F_SEAL_WRITE|F_SEAL_FUTURE_WRITE;
> +
>         if ((seals & F_SEAL_WRITE) && !(*file_seals & F_SEAL_WRITE)) {
>                 error = mapping_deny_writable(file->f_mapping);
>                 if (error)
> @@ -295,12 +301,6 @@ int memfd_add_seals(struct file *file, unsigned int seals)
>                 }
>         }
>
> -       /*
> -        * SEAL_EXEC implies SEAL_WRITE, making W^X from the start.
> -        */
> -       if (seals & F_SEAL_EXEC && inode->i_mode & 0111)
> -               seals |= F_SEAL_SHRINK|F_SEAL_GROW|F_SEAL_WRITE|F_SEAL_FUTURE_WRITE;
> -
>         *file_seals |= seals;
>         error = 0;
>
> --
> 2.54.0.545.g6539524ca2-goog
>

Re: [PATCH] memfd: deny writeable mappings when implying SEAL_WRITE

[David Hildenbrand (Arm)]

On 5/5/26 15:39, Pratyush Yadav wrote:
> From: "Pratyush Yadav (Google)" <pratyush@kernel.org>
> 
> When SEAL_EXEC is added, SEAL_WRITE is implied to make W^X. 

I don't quite understand that.

I guess what you mean is "SEAL_EXEC implies SEAL_WRITE if the file is
executable, to prevent W^X after sealing".

Because if the file is not executable, there is no sealing of writes happening?

It's rather odd to combine both things, though. Likely the callers should just
have requested SEAL_WRITE.

But I guess we are stuck with this mess.


> But the
> implied seal is set after the check that makes sure the memfd can not
> have any writable mappings. This means one can use SEAL_EXEC to apply
> SEAL_WRITE while having writeable mappings.
> 
> This breaks the contract that SEAL_WRITE provides and can be used by an
> attacker to pass a memfd that appears to be write sealed but can still
> be modified arbitrarily.
> 
> Fix this by adding the implied seals before the call for
> mapping_deny_writable() is done.
> 
> Fixes: c4f75bc8bd6b ("mm/memfd: add write seals when apply SEAL_EXEC to executable memfd")
> Cc: stable@vger.kernel.org
> Signed-off-by: Pratyush Yadav (Google) <pratyush@kernel.org>
> ---
>  mm/memfd.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/mm/memfd.c b/mm/memfd.c
> index fb425f4e315f..abe13b291ddc 100644
> --- a/mm/memfd.c
> +++ b/mm/memfd.c
> @@ -283,6 +283,12 @@ int memfd_add_seals(struct file *file, unsigned int seals)
>  		goto unlock;
>  	}
>  
> +	/*
> +	 * SEAL_EXEC implies SEAL_WRITE, making W^X from the start.
> +	 */
> +	if (seals & F_SEAL_EXEC && inode->i_mode & 0111)
> +		seals |= F_SEAL_SHRINK|F_SEAL_GROW|F_SEAL_WRITE|F_SEAL_FUTURE_WRITE;
> +
>  	if ((seals & F_SEAL_WRITE) && !(*file_seals & F_SEAL_WRITE)) {
>  		error = mapping_deny_writable(file->f_mapping);
>  		if (error)
> @@ -295,12 +301,6 @@ int memfd_add_seals(struct file *file, unsigned int seals)
>  		}
>  	}
>  
> -	/*
> -	 * SEAL_EXEC implies SEAL_WRITE, making W^X from the start.
> -	 */
> -	if (seals & F_SEAL_EXEC && inode->i_mode & 0111)
> -		seals |= F_SEAL_SHRINK|F_SEAL_GROW|F_SEAL_WRITE|F_SEAL_FUTURE_WRITE;
> -
>  	*file_seals |= seals;
>  	error = 0;
>  

Given the weird semantics, this makes sense to me.

Do we have to update documentation to reflect this? But staring at the man page
[1] we don't even seem to document F_SEAL_EXEC?


[1] https://www.man7.org/linux/man-pages/man2/F_ADD_SEALS.2const.html

-- 
Cheers,

David