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.

Sign up for GitHub

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

Jump to bottom

vm/{adb,isolated}: fail to clean up temp files #2831

Open
dvyukov opened this issue Oct 20, 2021 · 4 comments · May be fixed by #4372
Open

vm/{adb,isolated}: fail to clean up temp files #2831

dvyukov opened this issue Oct 20, 2021 · 4 comments · May be fixed by #4372

Comments

@dvyukov
Copy link
Collaborator

dvyukov commented Oct 20, 2021

vm/adb does "rm -Rf /data/syzkaller*" between reboots:

syzkaller/vm/adb/adb.go

Line 161 in 418a00e

if _, err := inst.adb("shell", "rm -Rf /data/syzkaller*"); err != nil {

because devices are not re-imaged between crashes (removing files at least frees up some space).

rm can't remove everything and it started failing recently after cgroup changes:

2021/10/18 12:20:27 failed to create instance: failed to run ["adb" "-s" "localhost:34369" 
rm: /data/syzkaller-testdir107020063/syzkaller.kw5FVG/233/cgroup: No such file or directory
rm: 233: Directory not empty

#2827 fixes this by unlinking symlinks.
But we need to do this for other VM impls (not really specific to adb) and use the more comprehensive executor remove_dir logic:

syzkaller/executor/common_linux.h

Line 4169 in 24dc29d

static void remove_dir(const char* dir)

I think we need remove these dirs as part of executor "setup" invocation (it runs exactly once after boot). We probably need a separate setup verb ("purge") because some tools (syz-execprog?) may not need this on every invocation.
@dvyukov dvyukov mentioned this issue Oct 20, 2021
Merged
@tr4v3ler
Copy link

tr4v3ler commented Aug 23, 2023 •
edited

I have encountered a similar problem:

failed to create instance: failed to run ["adb" "-s" "XXXX" "shell" "find /data/syzkaller* -type l -exec unlink {} \\; && rm -Rf /data/syzkaller*"]: exit status 1

The temporary solution is:

adb -s XXXX shell "chattr -R -ai /data/syzkaller* && rm -rf /data/syzkaller*"

@tr4v3ler
Copy link

tr4v3ler commented Aug 24, 2023 •
edited

These problems occur frequently, are there currently any ways to avoid them?

2023/08/24 15:11:03 failed to create instance: failed to run ["adb" "-s" "XXXX" "shell" "find /data/syzkaller* -type l -exec unlink {} \\; && rm -Rf /data/syzkaller*"]: exit status 1
find: /data/syzkaller-testdir3922641642/syzkaller.O4H8gj/1793/binderfs: No such file or directory

@tr4v3ler tr4v3ler mentioned this issue Sep 1, 2023
Open
@ramosian-glider
Copy link
Member

The following patch seems to fix the problem:

diff --git a/vm/adb/adb.go b/vm/adb/adb.go
index a108104c5..b87d50f5a 100644
--- a/vm/adb/adb.go
+++ b/vm/adb/adb.go
@@ -160,6 +160,10 @@ func (pool *Pool) Create(workdir string, index int) (vmimpl.Instance, error) {
        // Remove temp files from previous runs.
        // rm chokes on bad symlinks so we must remove them first
        if _, err := inst.adb("shell", "ls /data/syzkaller*"); err == nil {
+               if _, err := inst.adb("shell", "find /data/syzkaller* 2>&1 | grep 'No such file' "+
+                       "| sed 's/.*\\/data/\\/data/;s/:.*//' | xargs -r unlink"); err != nil {
+                       return nil, err
+               }
                if _, err := inst.adb("shell", "find /data/syzkaller* -type l -exec unlink {} \\;"+
                        " && rm -Rf /data/syzkaller*"); err != nil {
                        return nil, err

I am not sure the find ... -exec unlink part is needed anymore.

@ramosian-glider
Copy link
Member

ramosian-glider commented Dec 1, 2023 •
edited

@a-nogikh was curious what the broken file looks like:

/data/syzkaller-testdir3889977761/syzkaller.Ecm1Vc/4934 # ls
ls: ./cgroup: No such file or directory
/data/syzkaller-testdir3889977761/syzkaller.Ecm1Vc/4934 # file cgroup
cgroup: broken symbolic link to (null)

@ramosian-glider ramosian-glider linked a pull request Dec 1, 2023 that will close this issue
Open
ramosian-glider added a commit that referenced this issue Dec 1, 2023
@ramosian-glider
vm/adb: a more reliable way to delete broken symlinks
78b0f6e 
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

vm/adb: a more reliable way to delete broken symlinks google/syzkaller
3 participants
@ramosian-glider @dvyukov @tr4v3ler