| Message ID | 20210420165001.3790670-3-Liam.Howlett@Oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] arm64: armv8_deprecated: Fix swp_handler() signal generation | expand |
On Tue, Apr 20, 2021 at 04:50:13PM +0000, Liam Howlett wrote: > find_vma() will continue to search upwards until the end of the virtual > memory space. This means the si_code would almost never be set to > SEGV_MAPERR even when the address falls outside of any VMA. The result > is that the si_code is not reliable as it may or may not be set to the > correct result, depending on where the address falls in the address > space. > > Using find_vma_intersection() allows for what is intended by only > returning a VMA if it falls within the range provided, in this case a > window of 1. > > Fixes: bd35a4adc413 (arm64: Port SWP/SWPB emulation support from arm) > Signed-off-by: Liam R. Howlett <Liam.Howlett@Oracle.com> > --- > arch/arm64/kernel/traps.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c > index a05d34f0e82a..a44007904a64 100644 > --- a/arch/arm64/kernel/traps.c > +++ b/arch/arm64/kernel/traps.c > @@ -383,9 +383,10 @@ void force_signal_inject(int signal, int code, unsigned long address, unsigned i > void arm64_notify_segfault(unsigned long addr) > { > int code; > + unsigned long ut_addr = untagged_addr(addr); > > mmap_read_lock(current->mm); > - if (find_vma(current->mm, untagged_addr(addr)) == NULL) > + if (find_vma_intersection(current->mm, ut_addr, ut_addr + 1) == NULL) > code = SEGV_MAPERR; > else > code = SEGV_ACCERR; I'm not seeing how this addresses VM_GROWSDOWN as Catalin mentioned before. Will
* Will Deacon <will@kernel.org> [210422 08:53]: > On Tue, Apr 20, 2021 at 04:50:13PM +0000, Liam Howlett wrote: > > find_vma() will continue to search upwards until the end of the virtual > > memory space. This means the si_code would almost never be set to > > SEGV_MAPERR even when the address falls outside of any VMA. The result > > is that the si_code is not reliable as it may or may not be set to the > > correct result, depending on where the address falls in the address > > space. > > > > Using find_vma_intersection() allows for what is intended by only > > returning a VMA if it falls within the range provided, in this case a > > window of 1. > > > > Fixes: bd35a4adc413 (arm64: Port SWP/SWPB emulation support from arm) > > Signed-off-by: Liam R. Howlett <Liam.Howlett@Oracle.com> > > --- > > arch/arm64/kernel/traps.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c > > index a05d34f0e82a..a44007904a64 100644 > > --- a/arch/arm64/kernel/traps.c > > +++ b/arch/arm64/kernel/traps.c > > @@ -383,9 +383,10 @@ void force_signal_inject(int signal, int code, unsigned long address, unsigned i > > void arm64_notify_segfault(unsigned long addr) > > { > > int code; > > + unsigned long ut_addr = untagged_addr(addr); > > > > mmap_read_lock(current->mm); > > - if (find_vma(current->mm, untagged_addr(addr)) == NULL) > > + if (find_vma_intersection(current->mm, ut_addr, ut_addr + 1) == NULL) > > code = SEGV_MAPERR; > > else > > code = SEGV_ACCERR; > > I'm not seeing how this addresses VM_GROWSDOWN as Catalin mentioned before. Sorry for not including the link before, but for context, I've added the URL for the lore conversation from before at the end of my message [1]. I thought this was resolved by the fact that the stack expansion would have already taken care of the VM_GROWSDOWN code path? Did I misunderstand what was said? I've modified the other paths to this function to avoid causing issues elsewhere and to hopefully do the necessary cleanup that Catalin said needed to be tidied. Link: https://lore.kernel.org/lkml/20210413180030.GA31164@arm.com/ Thanks, Liam
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c index a05d34f0e82a..a44007904a64 100644 --- a/arch/arm64/kernel/traps.c +++ b/arch/arm64/kernel/traps.c @@ -383,9 +383,10 @@ void force_signal_inject(int signal, int code, unsigned long address, unsigned i void arm64_notify_segfault(unsigned long addr) { int code; + unsigned long ut_addr = untagged_addr(addr); mmap_read_lock(current->mm); - if (find_vma(current->mm, untagged_addr(addr)) == NULL) + if (find_vma_intersection(current->mm, ut_addr, ut_addr + 1) == NULL) code = SEGV_MAPERR; else code = SEGV_ACCERR;
find_vma() will continue to search upwards until the end of the virtual memory space. This means the si_code would almost never be set to SEGV_MAPERR even when the address falls outside of any VMA. The result is that the si_code is not reliable as it may or may not be set to the correct result, depending on where the address falls in the address space. Using find_vma_intersection() allows for what is intended by only returning a VMA if it falls within the range provided, in this case a window of 1. Fixes: bd35a4adc413 (arm64: Port SWP/SWPB emulation support from arm) Signed-off-by: Liam R. Howlett <Liam.Howlett@Oracle.com> --- arch/arm64/kernel/traps.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)