Skip to content

Conversation

@p13l13d13
Copy link
Contributor

If localhost is not resolvable on a system, we might hang without any logging (for example in vm/vmimpl/UnusedTCPPort) or fail later. Since syzkaller uses localhost (and not something like 127.0.0.1) it's way better to notify user earlier if their localhost is not configured correctly.


P.S. I was running syzkaller on a system with empty /etc/hosts, so I've stuck inside UnusedTCPPort, because of unsolvable 'localhost'.

I guess fixing this ONLY inside the problematic function is not enough, so I've added a top-level check. However, I'm not familiar with the codebase, so please tell me if I need to add this check to more locations or somewhere else.

@google-cla
Copy link

google-cla bot commented Jan 2, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@p13l13d13 p13l13d13 force-pushed the add-check-localhost branch 2 times, most recently from c8ec4a6 to f9432e2 Compare January 2, 2025 13:18
If localhost is not resolvable on a system, we might run into a loop
(for example in vm/vmimpl/UnusedTCPPort) or
fail later. Since syzkaller uses localhost (and not something like
127.0.0.1) it's way better to notify user earlier if their 'localhost' is
not configured correctly.

Signed-off-by: Ivan Gulakov <gulakov@amazon.de>
@p13l13d13 p13l13d13 force-pushed the add-check-localhost branch from f9432e2 to f69c8c1 Compare January 2, 2025 14:13
@a-nogikh
Copy link
Collaborator

a-nogikh commented Jan 2, 2025

To be honest, not having the localhost host seems to be such a non-typical situation that it feels just wrong for syz-manager to be validating for cases like this. There are infinite ways for the host system to be misconfigured.

That being said, I agree that the syzkaller should not have silently hung in this situation.

We could try to make UnusedTCPPort() check availability on all interfaces at the same time (":" + port instead of "localhost:" + port), but that still leaves a corner case of having 0 networking interfaces on the system :)

We could use 127.0.0.1 instead of localhost, but there's again a potential corner case of a machine without ipv4 support. Though syzkaller would anyway fail on such a machine since it already uses 127.0.0.1 in a number of places.

We could modify UnusedTCPPort() to look more carefully at the returned error and only continue if the error is about the port being busy.

There are other places that rely on localhost, but AFAIK they all should already be more explicit, e.g. qemu VMs will fail on start and you'll see some error messages in the logs.

@p13l13d13
Copy link
Contributor Author

Well, I think that ::1 and 127.0.0.1 are pretty much standard ways to access loopback interfaces. If 127.0.0.1 and ::1 are not accessible on a machine, then it has a rare custom configuration and operators/developers should know about it, thus it's not a syzkaller's problem.

IMHO, UnusedTCPPort should be fixed anyway, cause there is potential for locking in it.

So, my proposal is:

  1. I fix the UnusedTCPPort to exit on some errors.
  2. I replace 'localhost' with 127.0.0.1.
  3. (opt) use ::1 if it's an IPv6 machine. Though, that would require a bigger rework with a helper function, etc.

@tarasmadan
Copy link
Collaborator

#3579 fixed the failures on machines with IPv6 only.
Please don't change to 127.0.0.1.

@p13l13d13 p13l13d13 closed this Jan 7, 2025
@p13l13d13 p13l13d13 deleted the add-check-localhost branch January 7, 2025 12:20
@p13l13d13
Copy link
Contributor Author

Replaced with #5658

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants