| Message ID | 20230819094806.14965-15-kariem.taha2.7@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Implement the mmap system call for FreeBSD. | expand |
On Sat, Aug 19, 2023 at 3:49 AM Karim Taha <kariem.taha2.7@gmail.com> wrote: > From: Stacey Son <sson@FreeBSD.org> > > Co-authored-by: Kyle Evans <kevans@FreeBSD.org> > > Signed-off-by: Stacey Son <sson@FreeBSD.org> > Signed-off-by: Kyle Evans <kevans@FreeBSD.org> > Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com> > --- > bsd-user/bsd-mem.h | 11 +++++++++++ > bsd-user/freebsd/os-syscall.c | 4 ++++ > 2 files changed, 15 insertions(+) > Reviewed-by: Warner Losh <imp@bsdimp.com> but see below > diff --git a/bsd-user/bsd-mem.h b/bsd-user/bsd-mem.h > index a6446a454c..68d79ac080 100644 > --- a/bsd-user/bsd-mem.h > +++ b/bsd-user/bsd-mem.h > @@ -89,4 +89,15 @@ static inline abi_long do_bsd_mprotect(abi_long arg1, > abi_long arg2, > return get_errno(target_mprotect(arg1, arg2, arg3)); > } > > +/* msync(2) */ > +static inline abi_long do_bsd_msync(abi_long addr, abi_long len, abi_long > flags) > +{ > + if (!access_ok(VERIFY_WRITE, addr, len)) { > + /* XXX Should be EFAULT, but FreeBSD seems to get this wrong. */ > I'd reword this here (and in the bsd-user fork): /* It seems odd, but POSIX wants this to be ENOMEM */ since it isn't FreeBSD getting it wrong, and this matches the behavior that's documented for Linux as well (though in less detail). POSIX open group appears to document ENOMEM for this. > + return -TARGET_ENOMEM; > + } > + > + return get_errno(msync(g2h_untagged(addr), len, flags)); > +} > + > #endif /* BSD_USER_BSD_MEM_H */ > diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c > index aea4e337ff..3871b15309 100644 > --- a/bsd-user/freebsd/os-syscall.c > +++ b/bsd-user/freebsd/os-syscall.c > @@ -499,6 +499,10 @@ static abi_long freebsd_syscall(void *cpu_env, int > num, abi_long arg1, > ret = do_bsd_mprotect(arg1, arg2, arg3); > break; > > + case TARGET_FREEBSD_NR_msync: /* msync(2) */ > + ret = do_bsd_msync(arg1, arg2, arg3); > + break; > + > #if defined(__FreeBSD_version) && __FreeBSD_version >= 1300048 > case TARGET_FREEBSD_NR_shm_open2: /* shm_open2(2) */ > ret = do_freebsd_shm_open2(arg1, arg2, arg3, arg4, arg5); > -- > 2.40.0 > >
On 8/19/23 02:47, Karim Taha wrote: > +static inline abi_long do_bsd_msync(abi_long addr, abi_long len, abi_long flags) > +{ > + if (!access_ok(VERIFY_WRITE, addr, len)) { I think this check is wrong. There's nothing in the kernel that requires writability, or even that the entire range be mapped. I think you want guest_range_valid_untagged to simply check that the bounds are ok. With that, Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
diff --git a/bsd-user/bsd-mem.h b/bsd-user/bsd-mem.h index a6446a454c..68d79ac080 100644 --- a/bsd-user/bsd-mem.h +++ b/bsd-user/bsd-mem.h @@ -89,4 +89,15 @@ static inline abi_long do_bsd_mprotect(abi_long arg1, abi_long arg2, return get_errno(target_mprotect(arg1, arg2, arg3)); } +/* msync(2) */ +static inline abi_long do_bsd_msync(abi_long addr, abi_long len, abi_long flags) +{ + if (!access_ok(VERIFY_WRITE, addr, len)) { + /* XXX Should be EFAULT, but FreeBSD seems to get this wrong. */ + return -TARGET_ENOMEM; + } + + return get_errno(msync(g2h_untagged(addr), len, flags)); +} + #endif /* BSD_USER_BSD_MEM_H */ diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c index aea4e337ff..3871b15309 100644 --- a/bsd-user/freebsd/os-syscall.c +++ b/bsd-user/freebsd/os-syscall.c @@ -499,6 +499,10 @@ static abi_long freebsd_syscall(void *cpu_env, int num, abi_long arg1, ret = do_bsd_mprotect(arg1, arg2, arg3); break; + case TARGET_FREEBSD_NR_msync: /* msync(2) */ + ret = do_bsd_msync(arg1, arg2, arg3); + break; + #if defined(__FreeBSD_version) && __FreeBSD_version >= 1300048 case TARGET_FREEBSD_NR_shm_open2: /* shm_open2(2) */ ret = do_freebsd_shm_open2(arg1, arg2, arg3, arg4, arg5);