tmpfs: tmpfs_statfs recursively removes files

[Oxore]

Steps to reproduce

Prerequisites

Linux environment. arm-none-eabi-gcc toolchain, gcc version 9.3.0. QEMU with ARM emulation.

NuttX commit: 5b83906

Apps commit: dcc620e

Preparation

Let's assume we have cloned NuttX repo into /nuttx and apps repo into /apps. Current working directory: /nuttx. Prepare config:

./tools/configure.sh -l -a ../apps lm3s6965-ek:nsh

Adjust configuration:

CONFIG_TIVA_ETHERNET=n
CONFIG_NET=n
CONFIG_FS_TMPFS=y
CONFIG_FS_PROCFS=y

Build:

CROSSDEV=arm-none-eabi- make

Run:

qemu-system-arm -M lm3s6965evb -kernel ./nuttx

Go to serial0 (Ctrl+Alt+3) and get the console.

Actual steps to reproduce

At nsh prompt type:

mkdir /tmp/dir
echo >/tmp/file
echo >/tmp/dir/file2
ls /tmp
ls /tmp/dir

You can see created files and directory. Now run the following:

df

or:

cat /proc/fs/usage

then run:

ls /tmp
ls /tmp/dir

There is no /tmp/dir/file2 anymore, but /tmp/file still exists, though. So files in /tmp always exist, but any directory in /tmp becomes recursively cleaned up.

I expect, that when I run df, all my files in /tmp and deeper do not get changed or deleted, but they disappear.

Possible fix or just pointing to a buggy line of code

When file /proc/fs/usage is being read, it's read op calls tmpfs_statfs, that has subsequent call to tmpfs_foreach, which contains unconditional recursive call to itself with tmpfs_free_callout value being passed to the argument callout. This is why all files in all directories of /tmp disappear, but files and directories in /tmp (the root of tmpfs) stay on.

Here is an "one line" fix:

diff --git a/fs/tmpfs/fs_tmpfs.c b/fs/tmpfs/fs_tmpfs.c
index 43fb0fc081..915ccb6aec 100644
--- a/fs/tmpfs/fs_tmpfs.c
+++ b/fs/tmpfs/fs_tmpfs.c
@@ -1322,7 +1322,7 @@ static int tmpfs_foreach(FAR struct tmpfs_directory_s *tdo,
            * action will be to delete the directory.
            */

-          ret = tmpfs_foreach(next, tmpfs_free_callout, NULL);
+          ret = tmpfs_foreach(next, callout, arg);
           if (ret < 0)
             {
               return -ECANCELED;

So...

Does it fix the bug? Yes, it looks like it fixes it. If we create more files we get values increased in df output for /tmp and all files in /tmp and in it's directories persist.

Does it introduce more bugs? I don't know.

How can I test it? I hope that original author has better understanding of what is going on here and how to properly fix the bug.

I can try to open a pull request and carefully adjust all the code around, if somebody explain how I test my changes or at least point the places where to put attention to.

[xiaoxiang781216]

@Oxore thanks for the detailed bug report and analysis. Your change looks correctly, could you open a PR?