On Mon, 29 Jan 2024 at 09:40, Linus Torvalds
<torva...@linux-foundation.org> wrote:
>
> eventfs_create_events_dir() seems to have the same bug with ti->flags,
> btw, but got the ti->private initialization right.
>
> Funnily enough, create_file() got this right. I don't even understand
> why create_dir() did what it did.
>
> IOW, I think the right fix is really just this:
Actually, I think you have another uninitialized field here too:
'dentry->d_fsdata'.
And it looks like both create_file and create_dir got that wrong, but
eventfs_create_events_dir() got it right.
So you *also* need to do that
dentry->d_fsdata = ei;
before you do the d_instantiate().
Now, from a quick look, all the d_fsdata accesses *do* seem to be
protected by the eventfs_mutex, except
(a) eventfs_create_events_dir() doesn't seem to take the mutex, but
gets the ordering right, so is ok
(b) create_dir_dentry() drops the mutex in the middle, so the mutex
doesn't actually serialize anything
Not dropping the mutex in create_dir_dentry() *will* fix this bug, but
honestly, I'd suggest that in addition to not dropping the mutex, you
just fix the ordering too.
IOW, just do that
dentry->d_fsdata = ei;
before you do d_instantiate(), and now accessing d_fsdata is just
always safe and doesn't even need the mutex.
The whole "initialize everything before exposing it to others" is
simply just a good idea.
Linus
