Messages in this thread | | | From | Huacai Chen <> | Date | Tue, 17 Aug 2021 15:38:58 +0800 | Subject | Re: [PATCH] futex: Fix fault_in_user_writeable() |
| |
Hi, Thomas,
On Tue, Aug 17, 2021 at 3:07 PM Thomas Gleixner <tglx@linutronix.de> wrote: > > On Tue, Aug 17 2021 at 09:53, Huacai Chen wrote: > > On Tue, Aug 17, 2021 at 3:03 AM Thomas Gleixner <tglx@linutronix.de> wrote: > >> That's surely one way to fix that. If that does not work for whatever > >> reason, then we really don't want this find_vma() hack there, but rather > >> something like: > > I don't know why find_vma() is unacceptable here, there is also > > find_vma() in fixup_user_fault(). > > Wrong. find_extend_vma() != find_vma(). Aside of that fixup_user_fault() > does way more than that. > > >> if (IS_ENABLED(CONFIG_ARCH_USER_FAULT_VOODOO) && get_user(&tmp, uaddr)) > >> return -EFAULT; > > > > get_user() may be better than find_vma(), but can we drop > > CONFIG_ARCH_USER_FAULT_VOODOO here? On those "W implies R" archs, > > get_user() always success, this can simplify the logic. > > For architectures which imply R fixup_user_fault() is way more > effinicient than taking the fault on get_user() and then invoking > fixup_user_fault() to ensure that the mapping is writeable. > > No, we are not making stuff less efficient just because of MIPS. >
We use this program to test MIPS and X86:
int main(int argc, char** argv) { int fd; void *ptr; int ret; int syscall_nr = 98;
fd = open("/dev/zero", O_RDWR); if (fd == -1) exit(EXIT_FAILURE);
ptr = mmap(NULL, 16384, PROT_WRITE, MAP_ANONYMOUS | MAP_SHARED, fd, 0); close(fd); /* * futex syscall nr: * x86_64: 202 * MIPS84: 5194 */ #ifdef __mips__ syscall_nr = 5194; #elif __x86_64__ syscall_nr = 202; #endif
ret = syscall(syscall_nr,ptr,FUTEX_LOCK_PI,0, NULL, NULL, 0,0); printf("syscall %d ret = %d\n",syscall_nr,ret);
return 0; }
On X86, it returns 0; on MIPS64 without patch, it hangs in kernel; on MIPS64 with this patch, it returns -1.
Then, I want to know, on "W implies R" archs (such as X86), should it return 0? Maybe return -1 is more reasonable? (because the VMA is marked as write-only). If this program should return -1, then I don't think this is a MIPS-specific problem.
Huacai
> Thanks, > > tglx
| |