vm/adb: a more reliable way to delete broken symlinks

[ramosian-glider]

When fuzzing Android, the executor sometimes leaves broken symlinks that point to non-existent directories. The command that adb.go was using to delete the leftover symlinks: find /data/syzkaller* -type l -exec unlink {} \; actually choked on such files and led to syzkaller rebooting the device indefinitely. Parse the output of find /data/syzkaller* to obtain the list of broken symlinks and pass them to unlink one by one.

Fixes #2831.

---

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

---

[codecov[bot]]

Codecov Report

Merging #4372 (78b0f6e) into master (f819d6f) will decrease coverage by 0.0%. The diff coverage is n/a.

Additional details and impacted files

see 1 file with indirect coverage changes

[a-nogikh]

There seems to be a special find flag that focuses on broken symlinks: -xtype l

https://www.gnu.org/software/findutils/manual/html_mono/find.html#index-_002dxtype

Did you try something like find /data -xtype l -delete? Or does Android not support it?

[ramosian-glider]

Android's find is part of Toolbox, it does not implement -xtype.

[dvyukov]

I still think we should use executor's remove_dir as part of setup procedure to delete these files. We are playing whack-a-mole and duplicating code in shell.

[a-nogikh]

We cannot completely prevent all these problems in syz-executor anyway simply because it (and the whole device) may crash at any moment. And after reboot we still have do deal with everything it has left, so I don't think there's any way around improving vm/adb.

[dvyukov]

We can do it after booting the machine roughly at the same time the current removal hack runs. Executor setup already runs after booting the machine, and the cleanup looks like a reasonable part of setup. We also need all of this logic for vm/isolated and any other vm types that does not create VMs from scratch (e.g. some proxyvm uses).

[ramosian-glider]

@dvyukov right now syz-executor doesn't know anything about the locations of temporary files on Android.

Are you suggesting that adb.go somehow runs syz-executor with the paths that are intended to be removed?

[dvyukov]

"syz-executor setup" is run in the same dir where all fuzzing syz-executor processes are started. The temp dir for syz-executor is ".", so implicitly it knows it. I think "setup" procedure could just delete all "./syzkaller-testdir*" files. We could export this path pattern from pkg/ipc and pass to executor to avoid duplication, or add comments that they both know about this path. Perhaps we can even test it by running "setup" w/o arguments for test OS.