vm/adb: Don't check for ret code on adb shell reboot

[sebastianene07]

Prevent the fuzzer from entering in an infinte loop of device reboots when the adb shell reboot command returns with an error code.

Fixes: #6598

---

Before sending a pull request, please review Contribution Guidelines:
https://github.com/google/syzkaller/blob/master/docs/contributing.md

---

[google-cla]

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.

[a-nogikh]

The only "successful" return code besides 0 would be 255, right?
I think we'd better explicitly check for those two then, so that we don't paper over other possible issues.

---

FWIW in case of a non-zero exit code, the error would be VerboseError:

syzkaller/pkg/osutil/osutil.go

Lines 80 to 84 in 5757a3d

	return output.Bytes(), &VerboseError{
	Err: retErr,
	Output: output.Bytes(),
	ExitCode: exitCode,
	}

[sebastianene07]

The only "successful" return code besides 0 would be 255, right? I think we'd better explicitly check for those two then, so that we don't paper over other possible issues.

For the moment yes but I don't know if we can rely on this on the long run. If someone decides to change the adb shell ret code from -1 to something else then it will fail again.

FWIW in case of a non-zero exit code, the error would be VerboseError:

syzkaller/pkg/osutil/osutil.go

Lines 80 to 84 in 5757a3d

	return output.Bytes(), &VerboseError{
	Err: retErr,
	Output: output.Bytes(),
	ExitCode: exitCode,
	}

Right, thanks for pointing out.

[a-nogikh]

For the moment yes but I don't know if we can rely on this on the long run. If someone decides to change the adb shell ret code from -1 to something else then it will fail again.

My concern here is that it could just silently stop working / start failing with those other error codes and we will not even notice it.

[sebastianene07]

For the moment yes but I don't know if we can rely on this on the long run. If someone decides to change the adb shell ret code from -1 to something else then it will fail again.

My concern here is that it could just silently stop working / start failing with those other error codes and we will not even notice it.

Understood, I updated the pull requested to check for those error codes.

[a-nogikh]

Thanks!
Please note the failing CI checks.

vm/adb/adb.go:399:7: QF1001: could apply De Morgan's law (staticcheck)
Error: QF1001: could apply De Morgan's law (staticcheck)
1 issues:
* staticcheck: 1
Error: Process completed with exit code 2.

[sebastianene07]

Thanks! Please note the failing CI checks.

vm/adb/adb.go:399:7: QF1001: could apply De Morgan's law (staticcheck)
Error: QF1001: could apply De Morgan's law (staticcheck)
1 issues:
* staticcheck: 1
Error: Process completed with exit code 2.

sure, fixed this and pushed another update