| Message ID | tencent_871D1B705162CE42D21BD1F86E0447B44709@qq.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | RISC-V: mm: correct mmap behavior in sv48 address space | expand |
On Mon, Jan 15, 2024 at 03:58:30AM +0800, Yangyu Chen wrote: > A commit add2cc6b6515 ("RISC-V: mm: Restrict address space for > sv39,sv48,sv57") from patch[1] restricts regular mmap return address in the > sv48 space if the address hint is not above the sv48 userspace address. > However, this commit treats the address wrong which only use sv48 if the > hint address is above sv48 user address space. Actually, it should use sv48 > if the address is above sv39 user address space. Moreover, the original It is not valid to use any address in sv48 in this case. It is designed to provide an address that is in sv39 to guarantee that the application is able to handle any address that is returned by mmap. If, for example, the application asks for the address 1<<42, it expects that all of the bits above 42 are not used. The address provided by mmap must not contain those bits, so it uses the sv39 address space. - Charlie > patch code looks very complex in logic, we can simplify it with min marco. > > [1]. https://lore.kernel.org/r/20230809232218.849726-2-charlie@rivosinc.com > > Signed-off-by: Yangyu Chen <cyy@cyyself.name> > --- > arch/riscv/include/asm/processor.h | 39 ++++++------------------------ > 1 file changed, 8 insertions(+), 31 deletions(-) > > diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/processor.h > index e1944ff0757a..7ead6a3e1f12 100644 > --- a/arch/riscv/include/asm/processor.h > +++ b/arch/riscv/include/asm/processor.h > @@ -9,6 +9,7 @@ > #include <linux/const.h> > #include <linux/cache.h> > #include <linux/prctl.h> > +#include <linux/minmax.h> > > #include <vdso/processor.h> > > @@ -18,37 +19,13 @@ > #define DEFAULT_MAP_WINDOW (UL(1) << (MMAP_VA_BITS - 1)) > #define STACK_TOP_MAX TASK_SIZE > > -#define arch_get_mmap_end(addr, len, flags) \ > -({ \ > - unsigned long mmap_end; \ > - typeof(addr) _addr = (addr); \ > - if ((_addr) == 0 || (IS_ENABLED(CONFIG_COMPAT) && is_compat_task())) \ > - mmap_end = STACK_TOP_MAX; \ > - else if ((_addr) >= VA_USER_SV57) \ > - mmap_end = STACK_TOP_MAX; \ > - else if ((((_addr) >= VA_USER_SV48)) && (VA_BITS >= VA_BITS_SV48)) \ > - mmap_end = VA_USER_SV48; \ > - else \ > - mmap_end = VA_USER_SV39; \ > - mmap_end; \ > -}) > - > -#define arch_get_mmap_base(addr, base) \ > -({ \ > - unsigned long mmap_base; \ > - typeof(addr) _addr = (addr); \ > - typeof(base) _base = (base); \ > - unsigned long rnd_gap = DEFAULT_MAP_WINDOW - (_base); \ > - if ((_addr) == 0 || (IS_ENABLED(CONFIG_COMPAT) && is_compat_task())) \ > - mmap_base = (_base); \ > - else if (((_addr) >= VA_USER_SV57) && (VA_BITS >= VA_BITS_SV57)) \ > - mmap_base = VA_USER_SV57 - rnd_gap; \ > - else if ((((_addr) >= VA_USER_SV48)) && (VA_BITS >= VA_BITS_SV48)) \ > - mmap_base = VA_USER_SV48 - rnd_gap; \ > - else \ > - mmap_base = VA_USER_SV39 - rnd_gap; \ > - mmap_base; \ > -}) > +#define arch_get_mmap_end(addr, len, flags) \ > + ((addr) >= DEFAULT_MAP_WINDOW ? STACK_TOP_MAX :\ > + min(DEFAULT_MAP_WINDOW, STACK_TOP_MAX)) > + > +#define arch_get_mmap_base(addr, base) \ > + ((addr) >= DEFAULT_MAP_WINDOW ? base :\ > + min(base, DEFAULT_MAP_WINDOW)) > > #else > #define DEFAULT_MAP_WINDOW TASK_SIZE > -- > 2.43.0 >
diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/processor.h index e1944ff0757a..7ead6a3e1f12 100644 --- a/arch/riscv/include/asm/processor.h +++ b/arch/riscv/include/asm/processor.h @@ -9,6 +9,7 @@ #include <linux/const.h> #include <linux/cache.h> #include <linux/prctl.h> +#include <linux/minmax.h> #include <vdso/processor.h> @@ -18,37 +19,13 @@ #define DEFAULT_MAP_WINDOW (UL(1) << (MMAP_VA_BITS - 1)) #define STACK_TOP_MAX TASK_SIZE -#define arch_get_mmap_end(addr, len, flags) \ -({ \ - unsigned long mmap_end; \ - typeof(addr) _addr = (addr); \ - if ((_addr) == 0 || (IS_ENABLED(CONFIG_COMPAT) && is_compat_task())) \ - mmap_end = STACK_TOP_MAX; \ - else if ((_addr) >= VA_USER_SV57) \ - mmap_end = STACK_TOP_MAX; \ - else if ((((_addr) >= VA_USER_SV48)) && (VA_BITS >= VA_BITS_SV48)) \ - mmap_end = VA_USER_SV48; \ - else \ - mmap_end = VA_USER_SV39; \ - mmap_end; \ -}) - -#define arch_get_mmap_base(addr, base) \ -({ \ - unsigned long mmap_base; \ - typeof(addr) _addr = (addr); \ - typeof(base) _base = (base); \ - unsigned long rnd_gap = DEFAULT_MAP_WINDOW - (_base); \ - if ((_addr) == 0 || (IS_ENABLED(CONFIG_COMPAT) && is_compat_task())) \ - mmap_base = (_base); \ - else if (((_addr) >= VA_USER_SV57) && (VA_BITS >= VA_BITS_SV57)) \ - mmap_base = VA_USER_SV57 - rnd_gap; \ - else if ((((_addr) >= VA_USER_SV48)) && (VA_BITS >= VA_BITS_SV48)) \ - mmap_base = VA_USER_SV48 - rnd_gap; \ - else \ - mmap_base = VA_USER_SV39 - rnd_gap; \ - mmap_base; \ -}) +#define arch_get_mmap_end(addr, len, flags) \ + ((addr) >= DEFAULT_MAP_WINDOW ? STACK_TOP_MAX :\ + min(DEFAULT_MAP_WINDOW, STACK_TOP_MAX)) + +#define arch_get_mmap_base(addr, base) \ + ((addr) >= DEFAULT_MAP_WINDOW ? base :\ + min(base, DEFAULT_MAP_WINDOW)) #else #define DEFAULT_MAP_WINDOW TASK_SIZE
A commit add2cc6b6515 ("RISC-V: mm: Restrict address space for sv39,sv48,sv57") from patch[1] restricts regular mmap return address in the sv48 space if the address hint is not above the sv48 userspace address. However, this commit treats the address wrong which only use sv48 if the hint address is above sv48 user address space. Actually, it should use sv48 if the address is above sv39 user address space. Moreover, the original patch code looks very complex in logic, we can simplify it with min marco. [1]. https://lore.kernel.org/r/20230809232218.849726-2-charlie@rivosinc.com Signed-off-by: Yangyu Chen <cyy@cyyself.name> --- arch/riscv/include/asm/processor.h | 39 ++++++------------------------ 1 file changed, 8 insertions(+), 31 deletions(-)