Re: [PATCH v2] eventfs: Test for ei->is_freed when accessing ei->dentry

[Naresh Kamboju]

On Mon, 30 Oct 2023 at 20:11, Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
>
> On Mon, 30 Oct 2023 12:37:08 +0530
> Naresh Kamboju <naresh.kamboju@xxxxxxxxxx> wrote:
>
> >
> > I have tested the linux-trace.git trace/core and run selftests ftrace
> > the reported kernel panic [1] & [2] has been fixed but found
>
> Good to know. Can I add "Tested-by" from you for that bug fix?

Please add,
Tested-by: Linux Kernel Functional Testing <lkft@xxxxxxxxxx>
Tested-by: Naresh Kamboju <naresh.kamboju@xxxxxxxxxx>

>
> > "general protection fault"  at  kernel/trace/trace_events.c:2439.
>
> Can you test with the below patch?

Applied this patch on top of linux-trace.git trace/core and test passed.
All the reported issues have been resolved.

>
> Also, can I ask what are you testing this on that makes it trigger so
> easily? As I'm not able to trigger these in my tests, even though they are
> indeed bugs.

I use following build artifacts and running selftests: ftrace: ftracetest-ktap

   kernel:
      url:
        https://storage.tuxsuite.com/public/linaro/naresh/builds/2XT84xYJIMmKApmqOOtKhnLdCyz/bzImage
    modules:
      url:
        https://storage.tuxsuite.com/public/linaro/naresh/builds/2XT84xYJIMmKApmqOOtKhnLdCyz/modules.tar.xz
      compression: xz
    nfsrootfs:
      url: https://storage.tuxboot.com/debian/bookworm/amd64/rootfs.tar.xz
 kselftest:
          url:
            https://storage.tuxsuite.com/public/linaro/naresh/builds/2XT84xYJIMmKApmqOOtKhnLdCyz/kselftest.tar.xz


Steps to test:
==========

1) Boot x86 machine with above bzImage, modules, rootfs and
copy kselftest.tar.xz from above url and unzip.
2) cd /opt/kselftests/default-in-kernel/ftrace
3) ./run_kselftest.sh  -c ftrace


# selftests: ftrace: ftracetest-ktap
# unlink: cannot unlink
'/opt/kselftests/default-in-kernel/ftrace/logs/latest': No such file
or directory
# TAP version 13
# 1..129
# ok 1 Basic trace file check
...
# # Totals: pass:126 faii:0 xfail:1 xpass:0 skip:1 error:1


Test logs:
https://lkft.validation.linaro.org/scheduler/job/6985018#L4742




>
> -- Steve
>
> diff --git a/fs/tracefs/event_inode.c b/fs/tracefs/event_inode.c
> index 7ad7496bd597..7a0b54ddda24 100644
> --- a/fs/tracefs/event_inode.c
> +++ b/fs/tracefs/event_inode.c
> @@ -609,7 +609,13 @@ static struct dentry *eventfs_root_lookup(struct inode *dir,
>                 entry = &ei->entries[i];
>                 if (strcmp(name, entry->name) == 0) {
>                         void *cdata = data;
> -                       r = entry->callback(name, &mode, &cdata, &fops);
> +                       mutex_lock(&eventfs_mutex);
> +                       /* If ei->is_freed, then the event itself may be too */
> +                       if (!ei->is_freed)
> +                               r = entry->callback(name, &mode, &cdata, &fops);
> +                       else
> +                               r = -1;
> +                       mutex_unlock(&eventfs_mutex);
>                         if (r <= 0)
>                                 continue;
>                         ret = simple_lookup(dir, dentry, flags);
> @@ -743,7 +749,13 @@ static int dcache_dir_open_wrapper(struct inode *inode, struct file *file)
>                 void *cdata = data;
>                 entry = &ei->entries[i];
>                 name = entry->name;
> -               r = entry->callback(name, &mode, &cdata, &fops);
> +               mutex_lock(&eventfs_mutex);
> +               /* If ei->is_freed, then the event itself may be too */
> +               if (!ei->is_freed)
> +                       r = entry->callback(name, &mode, &cdata, &fops);
> +               else
> +                       r = -1;
> +               mutex_unlock(&eventfs_mutex);
>                 if (r <= 0)
>                         continue;
>                 d = create_file_dentry(ei, i, parent, name, mode, cdata, fops, false);