Re: [PATCH v9 2/3] cachestat: implement cachestat syscall

On Sun, Feb 5, 2023 at 1:56 AM Arnd Bergmann [off-list ref] wrote:

On Fri, Feb 3, 2023, at 20:04, Nhat Pham wrote:

quoted

+SYSCALL_DEFINE5(cachestat, unsigned int, fd, loff_t, off, size_t, len,
+             struct cachestat __user *, cstat, unsigned int, flags)
+{
+     return ksys_cachestat(fd, off, len, cstat, flags);
+}
+
+#ifdef CONFIG_COMPAT
+COMPAT_SYSCALL_DEFINE6(cachestat, unsigned int, fd,
compat_arg_u64_dual(off),
+             size_t, len, struct cachestat __user *, cstat, unsigned int, flags)
+{
+     return ksys_cachestat(fd, compat_arg_u64_glue(off), len, cstat,
flags);
+}

This still looks wrong to me, as this compat definition does not match
the native variant on architectures that require 64-bit arguments to
be passed in aligned register pairs, such as arm, mips or ppc, but
not x86, s390 or riscv.

(looks like I sent the last email in the wrong format - resending this
in plain text to see if the problem persists...)

Oh I see - thanks for pointing that out! And the last bit means this
is a non-issue for x86, s390 or riscv right?

And iirc from the last thread, this is fixable via a simple reordering
of the args in order to properly align the 64-bit arguments, for e.g:

SYSCALL_DEFINE5(cachestat, loff_t, off, unsigned int, fd, size_t, len,
              struct cachestat __user *, cstat, unsigned int, flags)

...

COMPAT_SYSCALL_DEFINE6(cachestat, compat_arg_u64_dual(off),
             unsigned int, fd, size_t, len, struct cachestat __user *, cstat,
             unsigned int, flags)


It looks a bit odd to me that fd is not the first argument, but perhaps this
is an acceptable sacrifice to avoid unused arg and keep the flags...

Let me know what you think about this!

     Arnd