8806 xattr_dir_inactive() releases used vnode with kernel panic

Review Request #789 — Created Dec. 4, 2017 and submitted June 21, 2018, 2:24 p.m.

Information
Owner:	vgusev
Repository:	illumos-gate
Branch:	master
Bugs:	8806
Depends On:
Commit:	3ae7d8f...
Reviewers
Groups:	general
People:	gwr

Description

8806 xattr_dir_inactive() releases used vnode with kernel panic

Testing Done

Testing for 12+ hours based on reproducer at #8806, no crashes.

Issues

0
0
2
0
All Issues: 2

Description	From	Last Updated
A comment here would be nice. Maybe: Note: gfs_dir_inactive returns NULL when we've lost a race with VGET (node is ...	gwr	Dec. 4, 2017, 5:10 p.m.
Please add != NULL here.	marcel	Dec. 5, 2017, 10:50 p.m.

```
Otherwise looks good.
```

usr/src/uts/common/fs/xattr.c (Diff revision 1)

Show all issues

A comment here would be nice. Maybe:

Note: gfs_dir_inactive returns NULL when we've
lost a race with VGET (node is still active)
otherwise this is really the last ref.

vgusev Dec. 4, 2017, 5:10 p.m.

Let me just add comment " vp was freed "

vgusev Dec. 4, 2017, 5:11 p.m.

It is common usage of gfs_dir_inactive().

Change Summary:

Added "/ vp was freed /" comment.

Commit:

+	3ae7d8f0cf9e9c3a0033a23859aebfa04b7a2021

Diff:

Revision 2 (+8 -15)

Show changes

usr/src/uts/common/fs/xattr.c

Ship it!

usr/src/uts/common/fs/xattr.c (Diff revision 2)
Show all issues
```
Please add != NULL here.
```

usr/src/uts/common/fs/xattr.c (Diff revision 2)

I'm not very happy with this typecast, but explicit dp->xattr_gfs_private.gfsd_file.gfs_size would be harder to read (but I still prefer it).

gwr Dec. 5, 2017, 5:39 p.m.

Or you could do something closer to what this consumer does:
http://src.illumos.org/source/xref/illumos-gate/usr/src/uts/common/fs/zfs/zfs_ctldir.c#1138
and use sizeof (xattr_dir_t)

That's the size we passed to gfs_dir_create.

usr/src/uts/common/fs/xattr.c (Diff revision 2)

Please let me leave as is because original code is:
kmem_free(fp, fp->gfs_size);
gfs_size should be used here, not direct size because gfs_size is set in gfs layer and in xattr layer it could be different.