FAILED: patch "[PATCH] mm: refactor map_deny_write_exec()" failed to apply to 6.6-stable tree
The patch below does not apply to the 6.6-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to stable@vger.kernel.org.
To reproduce the conflict and resubmit, you may use the following commands:
git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.6.y git checkout FETCH_HEAD git cherry-pick -x 0fb4a7ad270b3b209e510eb9dc5b07bf02b7edaf # <resolve conflicts, build, test, etc.> git commit -s git send-email --to 'stable@vger.kernel.org' --in-reply-to '2024111110-dubbed-hydration-c1be@gregkh' --subject-prefix 'PATCH 6.6.y' HEAD^..
Possible dependencies:
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From 0fb4a7ad270b3b209e510eb9dc5b07bf02b7edaf Mon Sep 17 00:00:00 2001 From: Lorenzo Stoakes lorenzo.stoakes@oracle.com Date: Tue, 29 Oct 2024 18:11:46 +0000 Subject: [PATCH] mm: refactor map_deny_write_exec()
Refactor the map_deny_write_exec() to not unnecessarily require a VMA parameter but rather to accept VMA flags parameters, which allows us to use this function early in mmap_region() in a subsequent commit.
While we're here, we refactor the function to be more readable and add some additional documentation.
Link: https://lkml.kernel.org/r/6be8bb59cd7c68006ebb006eb9d8dc27104b1f70.173022466... Fixes: deb0f6562884 ("mm/mmap: undo ->mmap() when arch_validate_flags() fails") Signed-off-by: Lorenzo Stoakes lorenzo.stoakes@oracle.com Reported-by: Jann Horn jannh@google.com Reviewed-by: Liam R. Howlett Liam.Howlett@oracle.com Reviewed-by: Vlastimil Babka vbabka@suse.cz Reviewed-by: Jann Horn jannh@google.com Cc: Andreas Larsson andreas@gaisler.com Cc: Catalin Marinas catalin.marinas@arm.com Cc: David S. Miller davem@davemloft.net Cc: Helge Deller deller@gmx.de Cc: James E.J. Bottomley James.Bottomley@HansenPartnership.com Cc: Linus Torvalds torvalds@linux-foundation.org Cc: Mark Brown broonie@kernel.org Cc: Peter Xu peterx@redhat.com Cc: Will Deacon will@kernel.org Cc: stable@vger.kernel.org Signed-off-by: Andrew Morton akpm@linux-foundation.org
diff --git a/include/linux/mman.h b/include/linux/mman.h index bcb201ab7a41..8ddca62d6460 100644 --- a/include/linux/mman.h +++ b/include/linux/mman.h @@ -188,16 +188,31 @@ static inline bool arch_memory_deny_write_exec_supported(void) * * d) mmap(PROT_READ | PROT_EXEC) * mmap(PROT_READ | PROT_EXEC | PROT_BTI) + * + * This is only applicable if the user has set the Memory-Deny-Write-Execute + * (MDWE) protection mask for the current process. + * + * @old specifies the VMA flags the VMA originally possessed, and @new the ones + * we propose to set. + * + * Return: false if proposed change is OK, true if not ok and should be denied. */ -static inline bool map_deny_write_exec(struct vm_area_struct *vma, unsigned long vm_flags) +static inline bool map_deny_write_exec(unsigned long old, unsigned long new) { + /* If MDWE is disabled, we have nothing to deny. */ if (!test_bit(MMF_HAS_MDWE, ¤t->mm->flags)) return false;
- if ((vm_flags & VM_EXEC) && (vm_flags & VM_WRITE)) + /* If the new VMA is not executable, we have nothing to deny. */ + if (!(new & VM_EXEC)) + return false; + + /* Under MDWE we do not accept newly writably executable VMAs... */ + if (new & VM_WRITE) return true;
- if (!(vma->vm_flags & VM_EXEC) && (vm_flags & VM_EXEC)) + /* ...nor previously non-executable VMAs becoming executable. */ + if (!(old & VM_EXEC)) return true;
return false; diff --git a/mm/mmap.c b/mm/mmap.c index ac0604f146f6..ab71d4c3464c 100644 --- a/mm/mmap.c +++ b/mm/mmap.c @@ -1505,7 +1505,7 @@ unsigned long mmap_region(struct file *file, unsigned long addr, vma_set_anonymous(vma); }
- if (map_deny_write_exec(vma, vma->vm_flags)) { + if (map_deny_write_exec(vma->vm_flags, vma->vm_flags)) { error = -EACCES; goto close_and_free_vma; } diff --git a/mm/mprotect.c b/mm/mprotect.c index 0c5d6d06107d..6f450af3252e 100644 --- a/mm/mprotect.c +++ b/mm/mprotect.c @@ -810,7 +810,7 @@ static int do_mprotect_pkey(unsigned long start, size_t len, break; }
- if (map_deny_write_exec(vma, newflags)) { + if (map_deny_write_exec(vma->vm_flags, newflags)) { error = -EACCES; break; } diff --git a/mm/vma.h b/mm/vma.h index 75558b5e9c8c..d58068c0ff2e 100644 --- a/mm/vma.h +++ b/mm/vma.h @@ -42,7 +42,7 @@ struct vma_munmap_struct { int vma_count; /* Number of vmas that will be removed */ bool unlock; /* Unlock after the munmap */ bool clear_ptes; /* If there are outstanding PTE to be cleared */ - /* 1 byte hole */ + /* 2 byte hole */ unsigned long nr_pages; /* Number of pages being removed */ unsigned long locked_vm; /* Number of locked pages */ unsigned long nr_accounted; /* Number of VM_ACCOUNT pages */
Refactor the map_deny_write_exec() to not unnecessarily require a VMA parameter but rather to accept VMA flags parameters, which allows us to use this function early in mmap_region() in a subsequent commit.
While we're here, we refactor the function to be more readable and add some additional documentation.
Reported-by: Jann Horn jannh@google.com Fixes: deb0f6562884 ("mm/mmap: undo ->mmap() when arch_validate_flags() fails") Cc: stable stable@kernel.org Reviewed-by: Liam R. Howlett Liam.Howlett@oracle.com Reviewed-by: Vlastimil Babka vbabka@suse.cz Reviewed-by: Jann Horn jannh@google.com Signed-off-by: Lorenzo Stoakes lorenzo.stoakes@oracle.com --- include/linux/mman.h | 21 ++++++++++++++++++--- mm/mmap.c | 2 +- mm/mprotect.c | 2 +- 3 files changed, 20 insertions(+), 5 deletions(-)
diff --git a/include/linux/mman.h b/include/linux/mman.h index db4741007bef..651705c2bf47 100644 --- a/include/linux/mman.h +++ b/include/linux/mman.h @@ -187,16 +187,31 @@ static inline bool arch_memory_deny_write_exec_supported(void) * * d) mmap(PROT_READ | PROT_EXEC) * mmap(PROT_READ | PROT_EXEC | PROT_BTI) + * + * This is only applicable if the user has set the Memory-Deny-Write-Execute + * (MDWE) protection mask for the current process. + * + * @old specifies the VMA flags the VMA originally possessed, and @new the ones + * we propose to set. + * + * Return: false if proposed change is OK, true if not ok and should be denied. */ -static inline bool map_deny_write_exec(struct vm_area_struct *vma, unsigned long vm_flags) +static inline bool map_deny_write_exec(unsigned long old, unsigned long new) { + /* If MDWE is disabled, we have nothing to deny. */ if (!test_bit(MMF_HAS_MDWE, ¤t->mm->flags)) return false;
- if ((vm_flags & VM_EXEC) && (vm_flags & VM_WRITE)) + /* If the new VMA is not executable, we have nothing to deny. */ + if (!(new & VM_EXEC)) + return false; + + /* Under MDWE we do not accept newly writably executable VMAs... */ + if (new & VM_WRITE) return true;
- if (!(vma->vm_flags & VM_EXEC) && (vm_flags & VM_EXEC)) + /* ...nor previously non-executable VMAs becoming executable. */ + if (!(old & VM_EXEC)) return true;
return false; diff --git a/mm/mmap.c b/mm/mmap.c index 9fefd13640d1..d71ac65563b2 100644 --- a/mm/mmap.c +++ b/mm/mmap.c @@ -2826,7 +2826,7 @@ unsigned long mmap_region(struct file *file, unsigned long addr, vma_set_anonymous(vma); }
- if (map_deny_write_exec(vma, vma->vm_flags)) { + if (map_deny_write_exec(vma->vm_flags, vma->vm_flags)) { error = -EACCES; goto close_and_free_vma; } diff --git a/mm/mprotect.c b/mm/mprotect.c index b94fbb45d5c7..7e870a8c9402 100644 --- a/mm/mprotect.c +++ b/mm/mprotect.c @@ -791,7 +791,7 @@ static int do_mprotect_pkey(unsigned long start, size_t len, break; }
- if (map_deny_write_exec(vma, newflags)) { + if (map_deny_write_exec(vma->vm_flags, newflags)) { error = -EACCES; break; }
On Thu, Nov 14, 2024 at 06:36:15PM +0000, Lorenzo Stoakes wrote:
Refactor the map_deny_write_exec() to not unnecessarily require a VMA parameter but rather to accept VMA flags parameters, which allows us to use this function early in mmap_region() in a subsequent commit.
While we're here, we refactor the function to be more readable and add some additional documentation.
Reported-by: Jann Horn jannh@google.com Fixes: deb0f6562884 ("mm/mmap: undo ->mmap() when arch_validate_flags() fails") Cc: stable stable@kernel.org Reviewed-by: Liam R. Howlett Liam.Howlett@oracle.com Reviewed-by: Vlastimil Babka vbabka@suse.cz Reviewed-by: Jann Horn jannh@google.com Signed-off-by: Lorenzo Stoakes lorenzo.stoakes@oracle.com
include/linux/mman.h | 21 ++++++++++++++++++--- mm/mmap.c | 2 +- mm/mprotect.c | 2 +- 3 files changed, 20 insertions(+), 5 deletions(-)
There's no clue here as to what the upstream git id is :(
Also, you sent lots of patches for each branch, but not as a series, so we have no idea what order these go in :(
Can you resend all of these, with the upstream git id in it, and as a patch series, so we know to apply them correctly?
thanks,
greg k-h
On Fri, Nov 15, 2024 at 05:02:29AM +0100, Greg KH wrote:
On Thu, Nov 14, 2024 at 06:36:15PM +0000, Lorenzo Stoakes wrote:
Refactor the map_deny_write_exec() to not unnecessarily require a VMA parameter but rather to accept VMA flags parameters, which allows us to use this function early in mmap_region() in a subsequent commit.
While we're here, we refactor the function to be more readable and add some additional documentation.
Reported-by: Jann Horn jannh@google.com Fixes: deb0f6562884 ("mm/mmap: undo ->mmap() when arch_validate_flags() fails") Cc: stable stable@kernel.org Reviewed-by: Liam R. Howlett Liam.Howlett@oracle.com Reviewed-by: Vlastimil Babka vbabka@suse.cz Reviewed-by: Jann Horn jannh@google.com Signed-off-by: Lorenzo Stoakes lorenzo.stoakes@oracle.com
include/linux/mman.h | 21 ++++++++++++++++++--- mm/mmap.c | 2 +- mm/mprotect.c | 2 +- 3 files changed, 20 insertions(+), 5 deletions(-)
There's no clue here as to what the upstream git id is :(
It's in-reply-to a mail that literally contains the upstream git id, following the instructions you explicitly gave.
Also, you sent lots of patches for each branch, but not as a series, so we have no idea what order these go in :(
I did wonder how you'd sort out ordering, but again, I was following your explicit instructions.
Can you resend all of these, with the upstream git id in it, and as a patch series, so we know to apply them correctly?
I'll do this, but... I do have to say, Greg, each of these patches are in reply to a mail stating something like, for instance this one:
The patch below does not apply to the 6.6-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to stable@vger.kernel.org.
(I note the above hand waves mention of including original git commit, but it's unwise to then immediately list explicit commands none of which mention this...)
To reproduce the conflict and resubmit, you may use the following commands:
git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.6.y git checkout FETCH_HEAD git cherry-pick -x 0fb4a7ad270b3b209e510eb9dc5b07bf02b7edaf # <resolve conflicts, build, test, etc.> git commit -s git send-email --to 'stable@vger.kernel.org' --in-reply-to '2024111110-dubbed-hydration-c1be@gregkh' --subject-prefix 'PATCH 6.6.y' HEAD^..
Might I politely suggest changing this or no longer telling people a series of commands to follow that result in 'please redo everything over again'?
Something like prefixing this with 'IF YOU NEED ONLY FIXUP A SINGLE COMMIT YOU CAN DO THE FOLLOWING:'?
Because right now it reads as 'you _must_ follow these instructions' to resolve the issue.
thanks,
greg k-h
A side note but... I didn't actually want to do these backports this way (as per our conversation prior to submission of original series), but my upstream patches got changed to cc: stable @ vger.kernel.org which triggered the bots, and is why I tried the follow these instructions.
I would otherwise have sent these as series in the first instance. But c'est la vie. Murphy's law dictated this series of events happen instead :(
Thanks, Lorenzo
On Fri, Nov 15, 2024 at 07:52:26AM +0000, Lorenzo Stoakes wrote:
On Fri, Nov 15, 2024 at 05:02:29AM +0100, Greg KH wrote:
On Thu, Nov 14, 2024 at 06:36:15PM +0000, Lorenzo Stoakes wrote:
Refactor the map_deny_write_exec() to not unnecessarily require a VMA parameter but rather to accept VMA flags parameters, which allows us to use this function early in mmap_region() in a subsequent commit.
While we're here, we refactor the function to be more readable and add some additional documentation.
Reported-by: Jann Horn jannh@google.com Fixes: deb0f6562884 ("mm/mmap: undo ->mmap() when arch_validate_flags() fails") Cc: stable stable@kernel.org Reviewed-by: Liam R. Howlett Liam.Howlett@oracle.com Reviewed-by: Vlastimil Babka vbabka@suse.cz Reviewed-by: Jann Horn jannh@google.com Signed-off-by: Lorenzo Stoakes lorenzo.stoakes@oracle.com
include/linux/mman.h | 21 ++++++++++++++++++--- mm/mmap.c | 2 +- mm/mprotect.c | 2 +- 3 files changed, 20 insertions(+), 5 deletions(-)
There's no clue here as to what the upstream git id is :(
It's in-reply-to a mail that literally contains the upstream git id, following the instructions you explicitly gave.
The instructions explicitly give you commands that say to use 'git cherry-pick -x' which adds the commit id :)
Also, you sent lots of patches for each branch, but not as a series, so we have no idea what order these go in :(
I did wonder how you'd sort out ordering, but again, I was following your explicit instructions.
Can you resend all of these, with the upstream git id in it, and as a patch series, so we know to apply them correctly?
I'll do this, but... I do have to say, Greg, each of these patches are in reply to a mail stating something like, for instance this one:
The patch below does not apply to the 6.6-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to stable@vger.kernel.org.
(I note the above hand waves mention of including original git commit, but it's unwise to then immediately list explicit commands none of which mention this...)
To reproduce the conflict and resubmit, you may use the following commands:
git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.6.y git checkout FETCH_HEAD git cherry-pick -x 0fb4a7ad270b3b209e510eb9dc5b07bf02b7edaf
See, -x, I think you forgot that :)
Anyway, this normally works just fine, as whole series of commits that fail are odd and rare. I can guess at ordering, like I do when I take them from Linus's tree (going by original commit dates), but for when you resend a bunch of them, it's much tricker as the original "FAILED" message doesn't show that order.
thanks,
greg k-h
On Fri, Nov 15, 2024 at 09:27:22AM +0100, Greg KH wrote:
On Fri, Nov 15, 2024 at 07:52:26AM +0000, Lorenzo Stoakes wrote:
On Fri, Nov 15, 2024 at 05:02:29AM +0100, Greg KH wrote:
On Thu, Nov 14, 2024 at 06:36:15PM +0000, Lorenzo Stoakes wrote:
Refactor the map_deny_write_exec() to not unnecessarily require a VMA parameter but rather to accept VMA flags parameters, which allows us to use this function early in mmap_region() in a subsequent commit.
While we're here, we refactor the function to be more readable and add some additional documentation.
Reported-by: Jann Horn jannh@google.com Fixes: deb0f6562884 ("mm/mmap: undo ->mmap() when arch_validate_flags() fails") Cc: stable stable@kernel.org Reviewed-by: Liam R. Howlett Liam.Howlett@oracle.com Reviewed-by: Vlastimil Babka vbabka@suse.cz Reviewed-by: Jann Horn jannh@google.com Signed-off-by: Lorenzo Stoakes lorenzo.stoakes@oracle.com
include/linux/mman.h | 21 ++++++++++++++++++--- mm/mmap.c | 2 +- mm/mprotect.c | 2 +- 3 files changed, 20 insertions(+), 5 deletions(-)
There's no clue here as to what the upstream git id is :(
It's in-reply-to a mail that literally contains the upstream git id, following the instructions you explicitly gave.
The instructions explicitly give you commands that say to use 'git cherry-pick -x' which adds the commit id :)
Right yes, missed that, these had to be hand fixed up which is part of it.
Also, you sent lots of patches for each branch, but not as a series, so we have no idea what order these go in :(
I did wonder how you'd sort out ordering, but again, I was following your explicit instructions.
Can you resend all of these, with the upstream git id in it, and as a patch series, so we know to apply them correctly?
I'll do this, but... I do have to say, Greg, each of these patches are in reply to a mail stating something like, for instance this one:
The patch below does not apply to the 6.6-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to stable@vger.kernel.org.
(I note the above hand waves mention of including original git commit, but it's unwise to then immediately list explicit commands none of which mention this...)
To reproduce the conflict and resubmit, you may use the following commands:
git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.6.y git checkout FETCH_HEAD git cherry-pick -x 0fb4a7ad270b3b209e510eb9dc5b07bf02b7edaf
See, -x, I think you forgot that :)
Anyway, this normally works just fine, as whole series of commits that fail are odd and rare. I can guess at ordering, like I do when I take them from Linus's tree (going by original commit dates), but for when you resend a bunch of them, it's much tricker as the original "FAILED" message doesn't show that order.
My point stands about rewording this, because I mean - I did what you asked (modulo mistakenly not getting the cherry-picked-by thing) - and it seems you are still unable to apply these.
I would add something about 'if there are a series to be applied, see <link to stable process option 3> or whatever it will be.
Because presumably, even if I had got the upstream commit ID bit right, you'd still have had no clue on ordering right? And we'd be in the same position.
thanks,
greg k-h
On Fri, Nov 15, 2024 at 08:38:00AM +0000, Lorenzo Stoakes wrote:
On Fri, Nov 15, 2024 at 09:27:22AM +0100, Greg KH wrote:
On Fri, Nov 15, 2024 at 07:52:26AM +0000, Lorenzo Stoakes wrote:
On Fri, Nov 15, 2024 at 05:02:29AM +0100, Greg KH wrote:
On Thu, Nov 14, 2024 at 06:36:15PM +0000, Lorenzo Stoakes wrote:
Refactor the map_deny_write_exec() to not unnecessarily require a VMA parameter but rather to accept VMA flags parameters, which allows us to use this function early in mmap_region() in a subsequent commit.
While we're here, we refactor the function to be more readable and add some additional documentation.
Reported-by: Jann Horn jannh@google.com Fixes: deb0f6562884 ("mm/mmap: undo ->mmap() when arch_validate_flags() fails") Cc: stable stable@kernel.org Reviewed-by: Liam R. Howlett Liam.Howlett@oracle.com Reviewed-by: Vlastimil Babka vbabka@suse.cz Reviewed-by: Jann Horn jannh@google.com Signed-off-by: Lorenzo Stoakes lorenzo.stoakes@oracle.com
include/linux/mman.h | 21 ++++++++++++++++++--- mm/mmap.c | 2 +- mm/mprotect.c | 2 +- 3 files changed, 20 insertions(+), 5 deletions(-)
There's no clue here as to what the upstream git id is :(
It's in-reply-to a mail that literally contains the upstream git id, following the instructions you explicitly gave.
The instructions explicitly give you commands that say to use 'git cherry-pick -x' which adds the commit id :)
Right yes, missed that, these had to be hand fixed up which is part of it.
Also, you sent lots of patches for each branch, but not as a series, so we have no idea what order these go in :(
I did wonder how you'd sort out ordering, but again, I was following your explicit instructions.
Can you resend all of these, with the upstream git id in it, and as a patch series, so we know to apply them correctly?
I'll do this, but... I do have to say, Greg, each of these patches are in reply to a mail stating something like, for instance this one:
The patch below does not apply to the 6.6-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to stable@vger.kernel.org.
(I note the above hand waves mention of including original git commit, but it's unwise to then immediately list explicit commands none of which mention this...)
To reproduce the conflict and resubmit, you may use the following commands:
git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.6.y git checkout FETCH_HEAD git cherry-pick -x 0fb4a7ad270b3b209e510eb9dc5b07bf02b7edaf
See, -x, I think you forgot that :)
Anyway, this normally works just fine, as whole series of commits that fail are odd and rare. I can guess at ordering, like I do when I take them from Linus's tree (going by original commit dates), but for when you resend a bunch of them, it's much tricker as the original "FAILED" message doesn't show that order.
My point stands about rewording this, because I mean - I did what you asked (modulo mistakenly not getting the cherry-picked-by thing) - and it seems you are still unable to apply these.
If I had the git ids, I'd take the time to figure it out. As-is, these were going to have to be redone anyway, hence me asking that they be sent in a series. That's all, just trying to make my life easier.
thanks,
greg k-h
Please ignore all of my series for 5.10.y, 5.15.y, 6.1.y, 6.6.y that I sent yesterday then, and I"ll resend _all_ just to make sure they're (ostensibly) correct.
Some have cherry picked upstream git id's some don't depending on whether I had to (painfully) manually fix up or (painfully) resolve merge conflicts, as I erroneously missed that -x added the (cherry picked...) comment.
This way I can make sure to cc- everybody consistently too rather than rely on the git send-email auto-cc.
Thanks.
linux-stable-mirror@lists.linaro.org
-
Greg KH -
gregkh＠linuxfoundation.org
-
Lorenzo Stoakes