diff mbox series

[2/2] vmalloc: reject vmap with VM_FLUSH_RESET_PERMS

Message ID 20221223092703.61927-3-hch@lst.de (mailing list archive)
State Not Applicable
Headers show
Series [1/2] Revert "remoteproc: qcom_q6v5_mss: map/unmap metadata region before/after use" | expand

Commit Message

Christoph Hellwig Dec. 23, 2022, 9:27 a.m. UTC
VM_FLUSH_RESET_PERMS is just for use with vmalloc as it is tied to freeing
the underlying pages.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 mm/vmalloc.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Lorenzo Stoakes Dec. 23, 2022, 10:24 a.m. UTC | #1
On Fri, Dec 23, 2022 at 10:27:03AM +0100, Christoph Hellwig wrote:
> VM_FLUSH_RESET_PERMS is just for use with vmalloc as it is tied to freeing
> the underlying pages.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  mm/vmalloc.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index 9e30f0b3920325..88a644cde9fb12 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -2849,6 +2849,9 @@ void *vmap(struct page **pages, unsigned int count,
>
>  	might_sleep();
>
> +	if (WARN_ON_ONCE(flags & VM_FLUSH_RESET_PERMS))
> +		return NULL;
> +

Might it be worth adding a specific vmap mask that explicitly indicates what
flags are permissible on vmap()? Then this could become e.g.:-

	if (WARN_ON_ONCE(flags & ~VM_VMAP_PERMITTED_MASK))
		return NULL;

And would be self-documenting as to why we are disallowing flags (i.e. they are
not part of the permitted vmap mask).
Christoph Hellwig Dec. 23, 2022, 2:03 p.m. UTC | #2
On Fri, Dec 23, 2022 at 10:24:25AM +0000, Lorenzo Stoakes wrote:
> Might it be worth adding a specific vmap mask that explicitly indicates what
> flags are permissible on vmap()? Then this could become e.g.:-
> 
> 	if (WARN_ON_ONCE(flags & ~VM_VMAP_PERMITTED_MASK))
> 		return NULL;
> 
> And would be self-documenting as to why we are disallowing flags (i.e. they are
> not part of the permitted vmap mask).

That's probably a good idea.  It might need some time to audit
for use of all the flags, though.
Lorenzo Stoakes Dec. 23, 2022, 2:10 p.m. UTC | #3
On Fri, Dec 23, 2022 at 03:03:12PM +0100, Christoph Hellwig wrote:
> On Fri, Dec 23, 2022 at 10:24:25AM +0000, Lorenzo Stoakes wrote:
> > Might it be worth adding a specific vmap mask that explicitly indicates what
> > flags are permissible on vmap()? Then this could become e.g.:-
> >
> > 	if (WARN_ON_ONCE(flags & ~VM_VMAP_PERMITTED_MASK))
> > 		return NULL;
> >
> > And would be self-documenting as to why we are disallowing flags (i.e. they are
> > not part of the permitted vmap mask).
>
> That's probably a good idea.  It might need some time to audit
> for use of all the flags, though.

Perhaps leave that for a later patch (I could take a look as well), but in the
meantime might be worth adding a quick comment here indicating why the flag is
prohibited?
diff mbox series

Patch

diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 9e30f0b3920325..88a644cde9fb12 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -2849,6 +2849,9 @@  void *vmap(struct page **pages, unsigned int count,
 
 	might_sleep();
 
+	if (WARN_ON_ONCE(flags & VM_FLUSH_RESET_PERMS))
+		return NULL;
+
 	/*
 	 * Your top guard is someone else's bottom guard. Not having a top
 	 * guard compromises someone else's mappings too.