net/sockopt::test_sockopts test failure on s390x about rustix HOT 10 CLOSED

Awesome, thank you! My test build of 0.33.6 was successful on x86_64, aarch64, and s390x.

from rustix.

Thanks for the report! This doesn't look related to any of the qemu bugs we've encountered, and this isn't expected. I don't currently have access to s390x hardware; I can test on qemu s390x-linux-user, though the test passes for me there.

Is it possible that Linux on s390x behaves differently than Linux on other platforms? qemu's linux-user feature is just a thin wrapper around the host OS, so it wouldn't necessarily reflect subtle differences.

Linux's socket support has a SO_BSDCOMPAT flag, which the documentation mentions "skipping of the broadcast flag". Is it possible that SO_BSDCOMPAT is enabled by default by Linux, or Fedora, on s390x?

Are the tests being run under seccomp? If so, it would be interesting to see whether the seccomp configuration modifies the behavior of socket, getsockopt, or setsockopt.

In case it's useful, the following is a standalone C program which tests the OS' behavior:

#include <sys/socket.h>
#include <assert.h>

int main(void) {
    int s = socket(AF_INET, SOCK_STREAM, 0);
    assert(s != -1);

    int one = 1;
    int r = setsockopt(s, SOL_SOCKET, SO_BROADCAST, &one, sizeof(one));
    assert(r != -1);

    int broadcast = -1;
    socklen_t optlen = (socklen_t)sizeof(broadcast);
    int q = getsockopt(s, SOL_SOCKET, SO_BROADCAST, &broadcast, &optlen);
    assert(q != -1);
    assert(optlen == sizeof(broadcast));
    assert(broadcast == 1);

    return 0;
}

from rustix.

Thanks for investigating! Having the small example program also should help me narrow down the problem. I think our build system might have seccomp filters set up for build containers (they use systemd-nspawn), so that could affect things, as well ... I'll report back once I know more.

from rustix.

This is weird: I added your test code into my build, compiled it with gcc, and ran it in the exact same environment as the rustix test suite. The program above returns with exit code 0 and does not print anything. The rustix test suite fails.

I compiled the C code with gcc -o test test.c and ran it with ./test. Is there some GCC flag I'm missing for assertions to work, or is that enough?

from rustix.

Ooh, I think I found a bug. Rustix is reading the value into a bool, rather than int. This may appear to "work" on little-endian platforms where the returned value of 1 is in the least-significant byte, but of course, it's a bug either way. I'll submit a PR to fix that.

My other idea here would be to ask if you could run the tests under strace -f -etrace=socket,getsockopt,setsockopt, but waiting for my patch is fine too.

from rustix.

I've now filed #280, which is likely to fix this issue. If you're able to test it now, that'd be great, but otherwise, I'll do a 0.34.x point release once it lands.

from rustix.

Ooh, I think I found a bug. Rustix is reading the value into a bool, rather than int. This may appear to "work" on little-endian platforms where the returned value of 1 is in the least-significant byte, but of course, it's a bug either way.

That indeed looks like the culprit 👍🏼

I tried backporting the patch from your PR to 0.33.5, but I gave up on that because it didn't apply cleanly, and I was out of my depth trying to manually apply the changes on top of unrelated changes in the getsockopt / setsockopt functions :( Given that the new wasmtime 0.36 branch is still using rustix 0.33.5, would it be possible to backport your changes to the 0.33 branch as well?

from rustix.

Will do. I've now cherry-picked the patch, along with a few other bug-fix patches, onto the v0.33.x branch. I'll run some tests to make sure everything still works, and then make a release.

from rustix.

Also, just for the record, as mentioned in #280, this bug doesn't involve out-of-bounds memory accesses. The kernel truncates the value it writes, rather than writing out of bounds, so it just produces the wrong value, on big-endian platforms.

from rustix.

@decathorpe 0.33.6 is now released, with the fix. I've now posted bytecodealliance/wasmtime#4022 to update Wasmtime.

from rustix.