diff mbox series

[v6,13/15] selftests/nolibc: add mmap_bad test case

Message ID fdc400ae881aa3819978d43cea7004df7b276551.1688739493.git.falcon@tinylab.org
State New
Headers
Series tools/nolibc: add a new syscall helper |

Commit Message

Zhangjin Wu July 7, 2023, 3:05 p.m. UTC 
  The length argument of mmap() must be greater than 0, passing a zero
length argument expects failure with -EINVAL.

Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
---
 tools/testing/selftests/nolibc/nolibc-test.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Willy Tarreau July 9, 2023, 9:15 a.m. UTC | #1 
On Fri, Jul 07, 2023 at 11:05:49PM +0800, Zhangjin Wu wrote:
> The length argument of mmap() must be greater than 0, passing a zero
> length argument expects failure with -EINVAL.

This one doesn't work for me on x86_64 kernel 5.15.112, qemu userland:

  46 mmap_bad = <0x0> EEXIST  != (<0xffffffffffffffff> EINVAL)    [FAIL]

This EEXIST actually is the errno from the previous test. If I run
the test natively it's OK:

  $ ./nolibc-test syscall:46
  Running test 'syscall'
  46 mmap_bad = <0xffffffffffffffff> EINVAL                        [OK]
  Errors during this test: 0

I'll queue it anyway for now but it would be nice that we figure what's
happening (even if we need to adjust or drop the test if it's a false
positive) so that we don't get used to "ah this is a normal error".

Willy
Zhangjin Wu July 9, 2023, 6:33 p.m. UTC | #2 
Hi, Willy

> On Fri, Jul 07, 2023 at 11:05:49PM +0800, Zhangjin Wu wrote:
> > The length argument of mmap() must be greater than 0, passing a zero
> > length argument expects failure with -EINVAL.
> 
> This one doesn't work for me on x86_64 kernel 5.15.112, qemu userland:
> 
>   46 mmap_bad = <0x0> EEXIST  != (<0xffffffffffffffff> EINVAL)    [FAIL]
>

Just rerun 'run-user' on x86_64 with kernel 5.11.0-41-generic, it is ok.

The failure is very interesting, and also rechecked the kernel mmap code:

    unsigned long do_mmap(struct file *file, unsigned long addr,
                        unsigned long len, unsigned long prot,
                        unsigned long flags, unsigned long pgoff,
                        unsigned long *populate, struct list_head *uf)
    {
        ...
        if (!len)
                return -EINVAL; 
        ...
    }

    $ git blame -L 1202,1202 mm/mmap.c
    e37609bb36f70 (Piotr Kwapulinski 2015-06-24 16:58:39 -0700 1202)        if (!len)

    $ git show e37609bb36f706c954e82e580e2e790e9d5caef8:Makefile 
    VERSION = 4
    PATCHLEVEL = 1
    SUBLEVEL = 0
    EXTRAVERSION =

So, the kernel side should be ok from v4.1?

For qemu-user, I have rechecked the following version:

    $ qemu-x86_64 --version
    qemu-x86_64 version 4.2.1 (Debian 1:4.2-3ubuntu6.18)

    $ qemu-x86_64 --version
    qemu-x86_64 version 7.0.0 (Debian 1:7.0+dfsg-7ubuntu2.6~backport20.04.202306190332~ubuntu20.04.1)

    $ build/x86_64/pc/qemu/v8.0.2/qemu-x86_64 --version
    qemu-x86_64 version 8.0.2 (v8.0.2-dirty)

all of them work well, as a comparison, what's your qemu-user version?

> This EEXIST actually is the errno from the previous test. If I run
> the test natively it's OK:
> 
>   $ ./nolibc-test syscall:46
>   Running test 'syscall'
>   46 mmap_bad = <0xffffffffffffffff> EINVAL                        [OK]
>   Errors during this test: 0
> 
> I'll queue it anyway for now but it would be nice that we figure what's
> happening (even if we need to adjust or drop the test if it's a false
> positive) so that we don't get used to "ah this is a normal error".
>

Yes, if there is a failure, we should figure out why. It is ok for me to remove
this one or let's find another errno condition before we find the root cause of
the reported failure. 

Thanks,
Zhangjin
 
> Willy
Willy Tarreau July 10, 2023, 7:04 a.m. UTC | #3 
Hi Zhangjin,

On Mon, Jul 10, 2023 at 02:33:22AM +0800, Zhangjin Wu wrote:
> For qemu-user, I have rechecked the following version:
> 
>     $ qemu-x86_64 --version
>     qemu-x86_64 version 4.2.1 (Debian 1:4.2-3ubuntu6.18)
> 
>     $ qemu-x86_64 --version
>     qemu-x86_64 version 7.0.0 (Debian 1:7.0+dfsg-7ubuntu2.6~backport20.04.202306190332~ubuntu20.04.1)
> 
>     $ build/x86_64/pc/qemu/v8.0.2/qemu-x86_64 --version
>     qemu-x86_64 version 8.0.2 (v8.0.2-dirty)
> 
> all of them work well, as a comparison, what's your qemu-user version?

Spot on! I ran it remotely and had a different qemu in my default path:

  $ qemu-x86_64 --version
  qemu-x86_64 version 2.7.0, Copyright (c) 2003-2016 Fabrice Bellard and the QEMU Project developers

I didn't notice but it yells at me during the test:

  qemu: Unsupported syscall: 332

Locally with this version it's much cleaner:

  $ qemu-x86_64 --version
  qemu-x86_64 version 6.2.0
  Copyright (c) 2003-2021 Fabrice Bellard and the QEMU Project developers
  $ make run-user | grep status:
  143 test(s): 141 passed,   2 skipped,   0 failed => status: warning

So we can keep it and blame my environment for that failure.

Thanks for checking,
Willy
diff mbox series

Patch

diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
index fde1b3c51a4a..4026772ed4c5 100644
--- a/tools/testing/selftests/nolibc/nolibc-test.c
+++ b/tools/testing/selftests/nolibc/nolibc-test.c
@@ -669,6 +669,7 @@  int run_syscall(int min, int max)
 		CASE_TEST(lseek_m1);          EXPECT_SYSER(1, lseek(-1, 0, SEEK_SET), -1, EBADF); break;
 		CASE_TEST(lseek_0);           EXPECT_SYSER(1, lseek(0, 0, SEEK_SET), -1, ESPIPE); break;
 		CASE_TEST(mkdir_root);        EXPECT_SYSER(1, mkdir("/", 0755), -1, EEXIST); break;
+		CASE_TEST(mmap_bad);          EXPECT_PTRER(1, mmap(NULL, 0, PROT_READ, MAP_PRIVATE, 0, 0), MAP_FAILED, EINVAL); break;
 		CASE_TEST(open_tty);          EXPECT_SYSNE(1, tmp = open("/dev/null", 0), -1); if (tmp != -1) close(tmp); break;
 		CASE_TEST(open_blah);         EXPECT_SYSER(1, tmp = open("/proc/self/blah", 0), -1, ENOENT); if (tmp != -1) close(tmp); break;
 		CASE_TEST(poll_null);         EXPECT_SYSZR(1, poll(NULL, 0, 0)); break;