diff mbox series

mm/mprotect: Fix do_mprotect_pkey() return on error

Message ID 20230406193050.1363476-1-Liam.Howlett@oracle.com
State New
Headers
Series mm/mprotect: Fix do_mprotect_pkey() return on error |

Commit Message

Liam R. Howlett April 6, 2023, 7:30 p.m. UTC 
  When the loop over the VMA is terminated early due to an error, the
return code could be overwritten with ENOMEM.  Fix the return code by
only setting the error on early loop termination when the error is not
set.

Fixes: 2286a6914c77 ("mm: change mprotect_fixup to vma iterator")
Cc: <stable@vger.kernel.org>
Signed-off-by: Liam R. Howlett <Liam.Howlett@oracle.com>
---
 mm/mprotect.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Andrew Morton April 6, 2023, 9:53 p.m. UTC | #1 
On Thu,  6 Apr 2023 15:30:50 -0400 "Liam R. Howlett" <Liam.Howlett@oracle.com> wrote:

> When the loop over the VMA is terminated early due to an error, the
> return code could be overwritten with ENOMEM.  Fix the return code by
> only setting the error on early loop termination when the error is not
> set.
> 
> Fixes: 2286a6914c77 ("mm: change mprotect_fixup to vma iterator")
> Cc: <stable@vger.kernel.org>

I do think we should always describe the user-visible effects when
proposing a backport.

a) so the -stable maintainers understand why we're recommending the
   backport and

b) to help some poor soul who is looking at the patch wondering if
   it will fix his customer's bug report.

How's this?

: User-visible effects include: attempts to run mprotect() against a special
: mapping or with a poorly-aligned hugetlb address should return -EINVAL,
: but they presently return -ENOMEM.
Liam R. Howlett April 6, 2023, 10:04 p.m. UTC | #2 
* Andrew Morton <akpm@linux-foundation.org> [230406 17:53]:
> On Thu,  6 Apr 2023 15:30:50 -0400 "Liam R. Howlett" <Liam.Howlett@oracle.com> wrote:
> 
> > When the loop over the VMA is terminated early due to an error, the
> > return code could be overwritten with ENOMEM.  Fix the return code by
> > only setting the error on early loop termination when the error is not
> > set.
> > 
> > Fixes: 2286a6914c77 ("mm: change mprotect_fixup to vma iterator")
> > Cc: <stable@vger.kernel.org>
> 
> I do think we should always describe the user-visible effects when
> proposing a backport.
> 
> a) so the -stable maintainers understand why we're recommending the
>    backport and
> 
> b) to help some poor soul who is looking at the patch wondering if
>    it will fix his customer's bug report.

Thanks, I'll keep this in mind.

> 
> How's this?
> 
> : User-visible effects include: attempts to run mprotect() against a special
> : mapping or with a poorly-aligned hugetlb address should return -EINVAL,
> : but they presently return -ENOMEM.

That sounds reasonable, although this isn't an exhaustive list. It could
be an -EACCESS for multiple reasons, or anything the vm_ops returns.
diff mbox series

Patch

diff --git a/mm/mprotect.c b/mm/mprotect.c
index 13e84d8c0797..36351a00c0e8 100644
--- a/mm/mprotect.c
+++ b/mm/mprotect.c
@@ -838,7 +838,7 @@  static int do_mprotect_pkey(unsigned long start, size_t len,
 	}
 	tlb_finish_mmu(&tlb);
 
-	if (vma_iter_end(&vmi) < end)
+	if (!error && vma_iter_end(&vmi) < end)
 		error = -ENOMEM;
 
 out: