Bug 203201

Summary: on zfs extattr behaviour broken after unlink
Product: Base System Reporter: justin
Component: kernAssignee: Andriy Gapon <avg>
Status: Closed FIXED    
Severity: Affects Some People CC: avg, fs, justin, mahrens, op
Priority: --- Keywords: patch
Version: CURRENTFlags: op: mfc-stable10?
Hardware: Any   
OS: Any   
Attachments:
Description Flags
test case
none
completely untested patch
none
test case when attribute exists none

Description justin 2015-09-18 19:19:59 UTC 
On ZFS root on
FreeBSD freebsd11 11.0-CURRENT FreeBSD 11.0-CURRENT #0 r285794: Wed Jul 22 16:58:53 UTC 2015     root@releng2.nyi.freebsd.org:/usr/obj/usr/src/sys/GENERIC  amd64

If I unlink a file, extattr_get_fd does not work correctly as can be seen from this ktrace:

  7638 luajit-2.0.4 CALL  openat(AT_FDCWD,0x107550,0x601<O_WRONLY|O_CREAT|O_TRUNC>,0x1c0<S_IRUSR|S_IWUSR|S_IXUSR>)
  7638 luajit-2.0.4 NAMI  "XXXXYYYYZZZ45217638"
  7638 luajit-2.0.4 RET   openat 3
  7638 luajit-2.0.4 CALL  unlink(0x107550)
  7638 luajit-2.0.4 NAMI  "XXXXYYYYZZZ45217638"
  7638 luajit-2.0.4 RET   unlink 0
  7638 luajit-2.0.4 CALL  extattr_get_fd(0x3,0x1,0x1129d8,0,0)
  7638 luajit-2.0.4 RET   extattr_get_fd -1 errno 2 No such file or directory

The correct behaviour is shown on a 10.2 system with ufs root:

 24254 luajit-2.0.3 CALL  open(0x106d48,0x601<O_WRONLY|O_CREAT|O_TRUNC>,0x1c0<S_IRUSR|S_IWUSR|S_IXUSR>)
 24254 luajit-2.0.3 NAMI  "XXXXYYYYZZZ452124254"
 24254 luajit-2.0.3 RET   open 3
 24254 luajit-2.0.3 CALL  unlink(0x106d48)
 24254 luajit-2.0.3 NAMI  "XXXXYYYYZZZ452124254"
 24254 luajit-2.0.3 RET   unlink 0
 24254 luajit-2.0.3 CALL  extattr_get_fd(0x3,0x1,0x1121d0,0,0)
 24254 luajit-2.0.3 RET   extattr_get_fd -1 errno 87 Attribute not found

The attribute not found error is the correct one as there are no xattrs on this new file, but somehow zfs is looking up the file and not finding it rather than using the file descriptor.

(there is a possibility that this is a regression from 10.2 to 11 but it seems more likely it is due to zfs; I dont have enough machines to test right now).
Comment 1 Andriy Gapon freebsd_committer freebsd_triage 2015-09-18 21:23:38 UTC 
I could be wrong but I think that ENOENT is returned not because the file is unlinked but because the attribute is not found.  That is, UFS reports the condition as errno 87, but ZFS reports it as errno 2.  That looks like a porting bug.  You can easily check that by calling extattr_get_fd(<missing-attribute>) on a file that is not unlinked.
Comment 2 justin 2015-09-18 21:26:10 UTC 
If the file is not unlinked I get attribute not found as expected (and as documented in the man page)

 11141 luajit-2.0.4 CALL  openat(AT_FDCWD,0x107550,0x601<O_WRONLY|O_CREAT|O_TRUNC>,0x1c0<S_IRUSR|S_IWUSR|S_IXUSR>)
 11141 luajit-2.0.4 NAMI  "XXXXYYYYZZZ452111141"
 11141 luajit-2.0.4 RET   openat 3
 11141 luajit-2.0.4 CALL  extattr_get_fd(0x3,0x1,0x1129d8,0,0)
 11141 luajit-2.0.4 RET   extattr_get_fd -1 errno 87 Attribute not found

So the behaviour is specifically related to unlinking, not the xattr behaviour.
Comment 3 Andriy Gapon freebsd_committer freebsd_triage 2015-09-18 21:32:26 UTC 
(In reply to justin from comment #2)
Thank you for the test.  I misread the code indeed.
Comment 4 justin 2015-09-19 09:07:30 UTC 
Created attachment 161182 [details]
test case

Test case.
Comment 5 Andriy Gapon freebsd_committer freebsd_triage 2015-09-19 09:48:27 UTC 
(In reply to justin from comment #4)
Could you please also test the case where the attribute is actually set?  That is, the case where extattr_get_fd() is supposed to succeed.  I think that that test would currently fail for the unlinked file with ENOENT as well.
Comment 6 Andriy Gapon freebsd_committer freebsd_triage 2015-09-19 09:52:43 UTC 
Created attachment 161183 [details]
completely untested patch
Comment 7 justin 2015-09-19 09:56:37 UTC 
Created attachment 161184 [details]
test case when attribute exists

Yes, when the attribute exists you get the same ENOENT error, see this test case.
Comment 8 Andriy Gapon freebsd_committer freebsd_triage 2015-10-30 12:54:55 UTC 
[ping]
Are you willing to test the patch?
Comment 9 commit-hook freebsd_committer freebsd_triage 2015-11-02 10:07:28 UTC 
A commit references this bug:

Author: avg
Date: Mon Nov  2 10:07:21 UTC 2015
New revision: 290266
URL: https://svnweb.freebsd.org/changeset/base/290266

Log:
  zfs: allow the lookup of extended attributes of an unlinked file

  That's required for extattr_get_fd(2) and the like to work properly.

  PR:		203201
  MFC after:	17 days

Changes:
  head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_dir.c
Comment 10 Oliver Pinter freebsd_committer freebsd_triage 2016-01-17 17:34:04 UTC 
MFC after:	17 days

Do you still want to MFC this change to 10-STABLE?
Comment 11 commit-hook freebsd_committer freebsd_triage 2016-03-20 23:21:17 UTC 
A commit references this bug:

Author: mav
Date: Sun Mar 20 23:20:16 UTC 2016
New revision: 297087
URL: https://svnweb.freebsd.org/changeset/base/297087

Log:
  MFC r290266 (by avg):
  zfs: allow the lookup of extended attributes of an unlinked file

  That's required for extattr_get_fd(2) and the like to work properly.

  PR:		203201

Changes:
_U  stable/10/
  stable/10/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_dir.c
Comment 12 Andriy Gapon freebsd_committer freebsd_triage 2016-03-24 11:42:18 UTC 
Thank you, Alexander.