Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:1 as permitted sender) client-ip=2620:137:e000::3:1;
Date:   Thu, 23 Nov 2023 12:25:48 +0100
From:   Heiko Carstens <hca@linux.ibm.com>
To:     Heiko Carstens <hca@linux.ibm.com>
Cc:     Steven Rostedt <rostedt@goodmis.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Linux Trace Kernel <linux-trace-kernel@vger.kernel.org>,
        Masami Hiramatsu <mhiramat@kernel.org>,
        Mark Rutland <mark.rutland@arm.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Ajay Kaher <akaher@vmware.com>, chinglinyu@google.com,
        lkp@intel.com, namit@vmware.com, oe-lkp@lists.linux.dev,
        amakhalov@vmware.com, er.ajay.kaher@gmail.com,
        srivatsa@csail.mit.edu, tkundu@vmware.com, vsirnapalli@vmware.com,
        linux-s390@vger.kernel.org
Subject: Re: [PATCH v5] eventfs: Remove eventfs_file and just use
 eventfs_inode
Message-ID: <20231123112548.9603-A-hca@linux.ibm.com>
References: <20231004165007.43d79161@gandalf.local.home>
 <20231117142335.9674-A-hca@linux.ibm.com>
 <20231117143829.9674-B-hca@linux.ibm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20231117143829.9674-B-hca@linux.ibm.com>
Precedence: bulk

On Fri, Nov 17, 2023 at 03:38:29PM +0100, Heiko Carstens wrote:
> On Fri, Nov 17, 2023 at 03:23:35PM +0100, Heiko Carstens wrote:
> > I think this patch causes from time to time crashes when running ftrace
> > selftests. In particular I guess there is a bug wrt error handling in this
> > function (see below for call trace):
> > 
> > > +static struct dentry *
> > > +create_file_dentry(struct eventfs_inode *ei, struct dentry **e_dentry,
> > > +		   struct dentry *parent, const char *name, umode_t mode, void *data,
> > > +		   const struct file_operations *fops, bool lookup)
> > > +{
> ...
> > Note that the compare and swap instruction within d_invalidate() generates
> > a specification exception because it operates on an invalid address
> > (0xffffffffffffffef), which happens to be -EEXIST. So my assumption is that
> > create_dir_dentry() has incorrect error handling and passes -EEXIST instead
> > of a valid dentry pointer to d_invalidate().
> > 
> > But I leave it up to you to figure this out :)
> 
> Ok, wrong function quoted of course. But the rest of my statement
> should be correct.

So, if it helps (this still happens with Linus' master branch):

create_dir_dentry() is called with a "struct eventfs_inode *ei" (second
parameter), which points to a data structure where "is_freed" is 1. Then it
looks like create_dir() returned "-EEXIST". And looking at the code this
combination then must lead to d_invalidate() incorrectly being called with
"-EEXIST" as dentry pointer.

Now, I have no idea how the code should work, but it is quite obvious that
something is broken :)

Here the dump of the struct eventfs_inode that was passed to
create_file_dentry() when the crash happened:

crash> struct eventfs_inode 00000000eada7680
struct eventfs_inode {
  list = {
    next = 0x10f802da0,
    prev = 0x122
  },
  entries = 0x12c031328 <event_entries>,
  name = 0x12b90bbac <__tpstrtab_xfs_alloc_vextent_exact_bno> "xfs_alloc_vextent_exact_bno",
  children = {
    next = 0xeada76a0,
    prev = 0xeada76a0
  },
  dentry = 0x0,
  d_parent = 0x107c75d40,
  d_children = 0xeada5700,
  entry_attrs = 0x0,
  attr = {
    mode = 0,
    uid = {
      val = 0
    },
    gid = {
      val = 0
    }
  },
  data = 0xeada6660,
  {
    llist = {
      next = 0xeada7668
    },
    rcu = {
      next = 0xeada7668,
      func = 0x12ad2a5b8 <free_rcu_ei>
    }
  },
  is_freed = 1,
  nr_entries = 6
}