Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mismatching manager/executor arches #5805

Open
ramosian-glider opened this issue Feb 19, 2025 · 4 comments
Open

mismatching manager/executor arches #5805

ramosian-glider opened this issue Feb 19, 2025 · 4 comments
Labels

Comments

@ramosian-glider
Copy link
Member

Starting Feb 14, I am seeing random local syz-manager crashes looking as follows:

2025/02/14 00:37:53 candidates=0 corpus=1735 coverage=13009 exec total=2206775 (46/sec) pending=1 reproducing=0 
2025/02/14 00:38:03 candidates=0 corpus=1735 coverage=13009 exec total=2207198 (46/sec) pending=1 reproducing=0 
2025/02/14 00:38:10 serv.handleConn returend mismatching manager/executor arches: arm64 vs 
2025/02/14 00:38:10 [FATAL] mismatching manager/executor arches: arm64 vs 

These are quite rare, but when I was running two syz-managers from two different checkouts in parallel, both died almost simultaneously (this happened two times, on Feb 14 and Feb 18).

@tarasmadan also saw a similar crash on an amd64 instance running overnight, and today we found occurrences of the same error messages (for both arm64 and amd64) in syzbot logs.

@ramosian-glider
Copy link
Member Author

The bug didn't reproduce yesterday when running syz-manager -mode=smoke-test several hundred times.

ramosian-glider added a commit to ramosian-glider/syzkaller that referenced this issue Feb 19, 2025
Dump the whole flatrpc.ConnectRequest to the logs, so that we can
better understand the cause of google#5805
ramosian-glider added a commit to ramosian-glider/syzkaller that referenced this issue Feb 19, 2025
Dump the whole flatrpc.ConnectRequest to the logs, so that we can
better understand the cause of google#5805
@dvyukov
Copy link
Collaborator

dvyukov commented Feb 19, 2025

One possible bug that may lead to this failure mode is some races on the connection buffer (updating/parsing it concurrently).

github-merge-queue bot pushed a commit that referenced this issue Feb 19, 2025
Dump the whole flatrpc.ConnectRequest to the logs, so that we can
better understand the cause of #5805
@ramosian-glider
Copy link
Member Author

One possible bug that may lead to this failure mode is some races on the connection buffer (updating/parsing it concurrently).

That's what Taras suspects as well. But it's strange that these races are so rare given the amount of connections).

@ramosian-glider
Copy link
Member Author

It turns out that a very simple program that connects to the manager's RPC socket and sends a TCP message to it may crash the manager if the message can be parsed as a flatbuffer.ConnectRequest (i.e. it starts with a proper length).

It's not completely clear which program is doing that, but to avoid crashes we need to ensure that the client is an executor before trusting it.

ramosian-glider added a commit to ramosian-glider/syzkaller that referenced this issue Feb 20, 2025
As we figured out in google#5805, syz-manager treats random incoming RPC
connections as trusted, and will crash if a non-executor client sends
an invalid packet to it.

To address this issue, we introduce another stage of handshake, which
includes a cookie exchange:
 - upon connection, the executor sends a ConnectHelloRequest containing
   its ID assigned to it at startup;
 - the manager responds with a ConnectHelloResponse containing a random
   64-bit cookie;
 - the executor calculates a hash of that cookie and includes it into
   its ConnectRequest together with the other information;
 - before checking the validity of ConnectRequest, the manager ensures
   client sanity (passed ID didn't change, hashed cookie has the expected
   value)

We deliberately pick a random cookie instead of a magic number: if the
fuzzer somehow learns to send packets to the manager, we don't want it to
crash multiple managers on the same machine.
ramosian-glider added a commit to ramosian-glider/syzkaller that referenced this issue Feb 20, 2025
As we figured out in google#5805, syz-manager treats random incoming RPC
connections as trusted, and will crash if a non-executor client sends
an invalid packet to it.

To address this issue, we introduce another stage of handshake, which
includes a cookie exchange:
 - upon connection, the executor sends a ConnectHelloRequest containing
   its ID assigned to it at startup;
 - the manager responds with a ConnectHelloResponse containing a random
   64-bit cookie;
 - the executor calculates a hash of that cookie and includes it into
   its ConnectRequest together with the other information;
 - before checking the validity of ConnectRequest, the manager ensures
   client sanity (passed ID didn't change, hashed cookie has the expected
   value)

We deliberately pick a random cookie instead of a magic number: if the
fuzzer somehow learns to send packets to the manager, we don't want it to
crash multiple managers on the same machine.
ramosian-glider added a commit to ramosian-glider/syzkaller that referenced this issue Feb 20, 2025
As we figured out in google#5805, syz-manager treats random incoming RPC
connections as trusted, and will crash if a non-executor client sends
an invalid packet to it.

To address this issue, we introduce another stage of handshake, which
includes a cookie exchange:
 - upon connection from an executor, the manager sends a ConnectHello RPC
   message to it, which contains a random 64-bit cookie;
 - the executor calculates a hash of that cookie and includes it into
   its ConnectRequest together with the other information;
 - before checking the validity of ConnectRequest, the manager ensures
   client sanity (passed ID didn't change, hashed cookie has the expected
   value)

We deliberately pick a random cookie instead of a magic number: if the
fuzzer somehow learns to send packets to the manager, we don't want it to
crash multiple managers on the same machine.
ramosian-glider added a commit to ramosian-glider/syzkaller that referenced this issue Feb 20, 2025
As we figured out in google#5805, syz-manager treats random incoming RPC
connections as trusted, and will crash if a non-executor client sends
an invalid packet to it.

To address this issue, we introduce another stage of handshake, which
includes a cookie exchange:
 - upon connection from an executor, the manager sends a ConnectHello RPC
   message to it, which contains a random 64-bit cookie;
 - the executor calculates a hash of that cookie and includes it into
   its ConnectRequest together with the other information;
 - before checking the validity of ConnectRequest, the manager ensures
   client sanity (passed ID didn't change, hashed cookie has the expected
   value)

We deliberately pick a random cookie instead of a magic number: if the
fuzzer somehow learns to send packets to the manager, we don't want it to
crash multiple managers on the same machine.
ramosian-glider added a commit to ramosian-glider/syzkaller that referenced this issue Feb 20, 2025
As we figured out in google#5805, syz-manager treats random incoming RPC
connections as trusted, and will crash if a non-executor client sends
an invalid packet to it.

To address this issue, we introduce another stage of handshake, which
includes a cookie exchange:
 - upon connection from an executor, the manager sends a ConnectHello RPC
   message to it, which contains a random 64-bit cookie;
 - the executor calculates a hash of that cookie and includes it into
   its ConnectRequest together with the other information;
 - before checking the validity of ConnectRequest, the manager ensures
   client sanity (passed ID didn't change, hashed cookie has the expected
   value)

We deliberately pick a random cookie instead of a magic number: if the
fuzzer somehow learns to send packets to the manager, we don't want it to
crash multiple managers on the same machine.
ramosian-glider added a commit to ramosian-glider/syzkaller that referenced this issue Feb 20, 2025
As we figured out in google#5805, syz-manager treats random incoming RPC
connections as trusted, and will crash if a non-executor client sends
an invalid packet to it.

To address this issue, we introduce another stage of handshake, which
includes a cookie exchange:
 - upon connection from an executor, the manager sends a ConnectHello RPC
   message to it, which contains a random 64-bit cookie;
 - the executor calculates a hash of that cookie and includes it into
   its ConnectRequest together with the other information;
 - before checking the validity of ConnectRequest, the manager ensures
   client sanity (passed ID didn't change, hashed cookie has the expected
   value)

We deliberately pick a random cookie instead of a magic number: if the
fuzzer somehow learns to send packets to the manager, we don't want it to
crash multiple managers on the same machine.
ramosian-glider added a commit to ramosian-glider/syzkaller that referenced this issue Feb 20, 2025
As we figured out in google#5805, syz-manager treats random incoming RPC
connections as trusted, and will crash if a non-executor client sends
an invalid packet to it.

To address this issue, we introduce another stage of handshake, which
includes a cookie exchange:
 - upon connection from an executor, the manager sends a ConnectHello RPC
   message to it, which contains a random 64-bit cookie;
 - the executor calculates a hash of that cookie and includes it into
   its ConnectRequest together with the other information;
 - before checking the validity of ConnectRequest, the manager ensures
   client sanity (passed ID didn't change, hashed cookie has the expected
   value)

We deliberately pick a random cookie instead of a magic number: if the
fuzzer somehow learns to send packets to the manager, we don't want it to
crash multiple managers on the same machine.
ramosian-glider added a commit to ramosian-glider/syzkaller that referenced this issue Feb 20, 2025
As we figured out in google#5805, syz-manager treats random incoming RPC
connections as trusted, and will crash if a non-executor client sends
an invalid packet to it.

To address this issue, we introduce another stage of handshake, which
includes a cookie exchange:
 - upon connection from an executor, the manager sends a ConnectHello RPC
   message to it, which contains a random 64-bit cookie;
 - the executor calculates a hash of that cookie and includes it into
   its ConnectRequest together with the other information;
 - before checking the validity of ConnectRequest, the manager ensures
   client sanity (passed ID didn't change, hashed cookie has the expected
   value)

We deliberately pick a random cookie instead of a magic number: if the
fuzzer somehow learns to send packets to the manager, we don't want it to
crash multiple managers on the same machine.
ramosian-glider added a commit to ramosian-glider/syzkaller that referenced this issue Feb 20, 2025
As we figured out in google#5805, syz-manager treats random incoming RPC
connections as trusted, and will crash if a non-executor client sends
an invalid packet to it.

To address this issue, we introduce another stage of handshake, which
includes a cookie exchange:
 - upon connection from an executor, the manager sends a ConnectHello RPC
   message to it, which contains a random 64-bit cookie;
 - the executor calculates a hash of that cookie and includes it into
   its ConnectRequest together with the other information;
 - before checking the validity of ConnectRequest, the manager ensures
   client sanity (passed ID didn't change, hashed cookie has the expected
   value)

We deliberately pick a random cookie instead of a magic number: if the
fuzzer somehow learns to send packets to the manager, we don't want it to
crash multiple managers on the same machine.
ramosian-glider added a commit to ramosian-glider/syzkaller that referenced this issue Feb 20, 2025
As we figured out in google#5805, syz-manager treats random incoming RPC
connections as trusted, and will crash if a non-executor client sends
an invalid packet to it.

To address this issue, we introduce another stage of handshake, which
includes a cookie exchange:
 - upon connection from an executor, the manager sends a ConnectHello RPC
   message to it, which contains a random 64-bit cookie;
 - the executor calculates a hash of that cookie and includes it into
   its ConnectRequest together with the other information;
 - before checking the validity of ConnectRequest, the manager ensures
   client sanity (passed ID didn't change, hashed cookie has the expected
   value)

We deliberately pick a random cookie instead of a magic number: if the
fuzzer somehow learns to send packets to the manager, we don't want it to
crash multiple managers on the same machine.
ramosian-glider added a commit to ramosian-glider/syzkaller that referenced this issue Feb 20, 2025
As we figured out in google#5805, syz-manager treats random incoming RPC
connections as trusted, and will crash if a non-executor client sends
an invalid packet to it.

To address this issue, we introduce another stage of handshake, which
includes a cookie exchange:
 - upon connection from an executor, the manager sends a ConnectHello RPC
   message to it, which contains a random 64-bit cookie;
 - the executor calculates a hash of that cookie and includes it into
   its ConnectRequest together with the other information;
 - before checking the validity of ConnectRequest, the manager ensures
   client sanity (passed ID didn't change, hashed cookie has the expected
   value)

We deliberately pick a random cookie instead of a magic number: if the
fuzzer somehow learns to send packets to the manager, we don't want it to
crash multiple managers on the same machine.
github-merge-queue bot pushed a commit that referenced this issue Feb 20, 2025
As we figured out in #5805, syz-manager treats random incoming RPC
connections as trusted, and will crash if a non-executor client sends
an invalid packet to it.

To address this issue, we introduce another stage of handshake, which
includes a cookie exchange:
 - upon connection from an executor, the manager sends a ConnectHello RPC
   message to it, which contains a random 64-bit cookie;
 - the executor calculates a hash of that cookie and includes it into
   its ConnectRequest together with the other information;
 - before checking the validity of ConnectRequest, the manager ensures
   client sanity (passed ID didn't change, hashed cookie has the expected
   value)

We deliberately pick a random cookie instead of a magic number: if the
fuzzer somehow learns to send packets to the manager, we don't want it to
crash multiple managers on the same machine.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants