I have found several problems concerning device and fifo files on ACL enabled UFS file systems. 1) Setting ACLs for block device files panics the system. r115588 disabled ACL support for character devices: ------------------------------------------------------------------------ r115588 | rwatson | 2003-06-01 05:42:18 +0300 (Sun, 01 Jun 2003) | 13 lines Return EOPNOTSUPP for attempted EA operations on VCHR vnodes in UFS2; if we permit them to occur, the kernel panics due to our performing EA operations using VOP_STRATEGY on the vnode. This went unnoticed previously because there are very for users of device nodes on UFS2 due to the introduction of devfs. However, this can come up with the Linux compat directories and its hard-coded dev nodes (which will need to go away as we move away from hard-coded device numbers). This can come up if you use EA-intensive features such as ACLs and MAC. The proper fix is pretty complicated, but this band-aid would be an excellent MFC candidate for the release. ------------------------------------------------------------------------ Apparently the same problem exists for VBLK vnodes but the commit above disabled support for VCHR vnodes only. Panic information is found below. The panic is not reproducible without INVARIANTS because the panic is caused by KASSERT in bufstrategy(): KASSERT(vp->v_type != VCHR && vp->v_type != VBLK, ("Wrong vnode in bufstrategy(bp=%p, vp=%p)", bp, vp)); Surprisingly a quick test suggests that ACLs may actually work for block devices if INVARIANTS option is not set. --- panic begins here --- panic: Wrong vnode in bufstrategy(bp=0xcd1e70f0, vp=0xc3203118) cpuid = 0 KDB: enter: panic panic: from debugger cpuid = 0 Uptime: 3m16s Physical memory: 499 MB Dumping 36 MB: 21 5 #0 doadump () at pcpu.h:196 196 pcpu.h: No such file or directory. in pcpu.h (kgdb) bt #0 doadump () at pcpu.h:196 #1 0xc07735be in boot (howto=260) at /home/jaakko/src/sys/kern/kern_shutdown.c:418 #2 0xc0773883 in panic (fmt=Variable "fmt" is not available. ) at /home/jaakko/src/sys/kern/kern_shutdown.c:572 #3 0xc0493ff7 in db_panic (addr=Could not find the frame base for "db_panic". ) at /home/jaakko/src/sys/ddb/db_command.c:446 #4 0xc04949fc in db_command (last_cmdp=0xc0c02f94, cmd_table=0x0, dopager=1) at /home/jaakko/src/sys/ddb/db_command.c:413 #5 0xc0494b0a in db_command_loop () at /home/jaakko/src/sys/ddb/db_command.c:466 #6 0xc04962fd in db_trap (type=3, code=0) at /home/jaakko/src/sys/ddb/db_main.c:228 #7 0xc079e086 in kdb_trap (type=3, code=0, tf=0xd61f07b8) at /home/jaakko/src/sys/kern/subr_kdb.c:510 #8 0xc0a8152b in trap (frame=0xd61f07b8) at /home/jaakko/src/sys/i386/i386/trap.c:643 #9 0xc0a64ffb in calltrap () at /home/jaakko/src/sys/i386/i386/exception.s:146 #10 0xc079e20a in kdb_enter (why=0xc0b0d6c6 "panic", msg=0xc0b0d6c6 "panic") at cpufunc.h:60 #11 0xc077386c in panic ( fmt=0xc0b174b6 "Wrong vnode in bufstrategy(bp=%p, vp=%p)") at /home/jaakko/src/sys/kern/kern_shutdown.c:556 #12 0xc07dd222 in bufstrategy (bo=0xc32031c8, bp=0xcd1e70f0) at /home/jaakko/src/sys/kern/vfs_bio.c:3808 #13 0xc07e2579 in bufwrite (bp=0xcd1e70f0) at buf.h:397 #14 0xc0979e7c in ffs_close_ea (vp=0xc3203118, commit=Variable "commit" is not available. ) at buf.h:385 #15 0xc097a2eb in ffs_setextattr (ap=0xd61f098c) at /home/jaakko/src/sys/ufs/ffs/ffs_vnops.c:1714 #16 0xc0a8b2f5 in VOP_SETEXTATTR_APV (vop=0xc0be94a0, a=0xd61f098c) at vnode_if.c:2660 #17 0xc080225a in vn_extattr_set (vp=0xc3203118, ioflg=Variable "ioflg" is not available. ) at vnode_if.h:1446 #18 0xc097b13e in ufs_setacl (ap=0xd61f0a60) at /home/jaakko/src/sys/ufs/ufs/ufs_acl.c:349 #19 0xc0a8b915 in VOP_SETACL_APV (vop=0xc0be94a0, a=0xd61f0a60) at vnode_if.c:2292 #20 0xc07dc057 in vacl_set_acl (td=0xc31ebaa0, vp=0xc3203118, type=0, aclp=0x8103560) at vnode_if.h:1219 #21 0xc07dc2d1 in __acl_set_file (td=0xc31ebaa0, uap=0xd61f0cfc) at /home/jaakko/src/sys/kern/vfs_acl.c:240 #22 0xc0a80cb3 in syscall (frame=0xd61f0d38) at /home/jaakko/src/sys/i386/i386/trap.c:1026 #23 0xc0a65060 in Xint0x80_syscall () at /home/jaakko/src/sys/i386/i386/exception.s:203 #24 0x00000033 in ?? () --- panic ends here --- 2) pathconf(2) _PC_ACL_EXTENDED returns EINVAL for fifos although fifos support ACLs (on ACL enabled file systems). For block and character devices pathconf(2) claims that ACLs can be set by returning 1. This is not true as described above. The same applies to _PC_ACL_PATH_MAX too. 3) ls(1) uses pathconf(2) to determinate if ACLs are supported. In printlong() ls(1) disables ACL checking when it encounters first file failing pathconf() _PC_ACL_EXTENDED call. However there might be some files supporting ACLs and some that doesn't even in the same directory. This combined with the pathconf(2) bugs described in 2) cause ls(1) to behave erratically. Fix: Here's a patch which makes following changes to UFS: - disable ACL support for VBLK vnodes (for VCHR nodes it's already disabled) - make ufs_pathconf() aware that ACLs are not supported for device files - implement pathconf VOP for fifos. ufsfifo_pathconf() returns proper values for _PC_ACL_EXTENDED and _PC_ACL_PATH_MAX I see two options to fix ls(1): 1) Remove pathconf(2) result caching completely. This increases number of pathconf(2) system calls on file systems with ACLs disabled. 2) Only make assumptions about file system supporting ACLs on regular files or directories. This patch implements 2): How-To-Repeat: (ACLs enabled) # mount|grep /img /dev/md0 on /img (ufs, local, acls) # cd /img Case 1: # mknod device2 b 1 1 # setfacl -m u:operator:r device2 (system panics in bufstrategy()) Cases 2 and 3: # mknod device c 1 1 # mkfifo fifo # touch file # setfacl -m u:operator:r file # setfacl -m u:operator:r fifo # ls -l total 12 drwxrwxr-x 2 root operator 512 Jul 8 15:20 .snap ls: ./device: Operation not supported crw-r--r-- 1 root wheel 1, 1 Jul 10 14:34 device prw-r--r-- 1 root wheel 0 Jul 10 14:34 fifo -rw-r--r-- 1 root wheel 0 Jul 10 14:34 file (ls should display "+" after mode string for files which have ACLs set) # ls -l file -rw-r--r--+ 1 root wheel 0 Jul 10 14:34 file # truss ls -l 2>&1|grep pathconf pathconf("./.snap",_PC_ACL_EXTENDED) = 1 (0x1) pathconf("./device",_PC_ACL_EXTENDED) = 1 (0x1) pathconf("./fifo",_PC_ACL_EXTENDED) ERR#22 'Invalid argument'
Responsible Changed From-To: freebsd-bugs->rwatson Grab ownership of this PR since I've done work in the ACL aread.
On Tue, 22 Jul 2008, Jaakko Heinonen wrote: > You are listed in src/MAINTAINERS for POSIX.1e ACLs and you have worked on > the code in question. Could you possibly look at following PR and the patch > attached to it? Dear Jaako: I'd be happy to take a look at this, and especially try and get the fixes merged before FreeBSD 7.1. I'm fairly busy for the next few days but will attempt to look at it over the weekend or next week. Thanks! Robert N M Watson Computer Laboratory University of Cambridge > > On 2008-07-14, FreeBSD-gnats-submit@FreeBSD.org wrote: >> You can access the state of your problem report at any time >> via this link: >> >> http://www.freebsd.org/cgi/query-pr.cgi?pr=125613 >> >>> Category: kern >>> Responsible: freebsd-bugs >>> Synopsis: [patch] ACL problems with special files >>> Arrival-Date: Mon Jul 14 19:40:05 UTC 2008 > > -- > Jaakko >
State Changed From-To: open->analyzed Despite some delays, I've finally gotten around to looking at this. I can confirm the panic and ls(1) problems, and the workaround looks reasonable. I will look at merging some or all of these changes in the next few days.
Author: trasz Date: Wed Jul 1 22:30:36 2009 New Revision: 195265 URL: http://svn.freebsd.org/changeset/base/195265 Log: Don't panic on attempt to set ACL on a block device file. This is just a part of kern/125613. PR: kern/125613 Submitted by: Jaakko Heinonen <jh at saunalahti dot fi> Reviewed by: rwatson Approved by: re (kib) Modified: head/sys/ufs/ffs/ffs_vnops.c Modified: head/sys/ufs/ffs/ffs_vnops.c ============================================================================== --- head/sys/ufs/ffs/ffs_vnops.c Wed Jul 1 22:23:26 2009 (r195264) +++ head/sys/ufs/ffs/ffs_vnops.c Wed Jul 1 22:30:36 2009 (r195265) @@ -1401,7 +1401,7 @@ struct vop_openextattr_args { ip = VTOI(ap->a_vp); fs = ip->i_fs; - if (ap->a_vp->v_type == VCHR) + if (ap->a_vp->v_type == VCHR || ap->a_vp->v_type == VBLK) return (EOPNOTSUPP); return (ffs_open_ea(ap->a_vp, ap->a_cred, ap->a_td)); @@ -1429,7 +1429,7 @@ struct vop_closeextattr_args { ip = VTOI(ap->a_vp); fs = ip->i_fs; - if (ap->a_vp->v_type == VCHR) + if (ap->a_vp->v_type == VCHR || ap->a_vp->v_type == VBLK) return (EOPNOTSUPP); if (ap->a_commit && (ap->a_vp->v_mount->mnt_flag & MNT_RDONLY)) @@ -1462,7 +1462,7 @@ vop_deleteextattr { ip = VTOI(ap->a_vp); fs = ip->i_fs; - if (ap->a_vp->v_type == VCHR) + if (ap->a_vp->v_type == VCHR || ap->a_vp->v_type == VBLK) return (EOPNOTSUPP); if (strlen(ap->a_name) == 0) @@ -1549,7 +1549,7 @@ vop_getextattr { ip = VTOI(ap->a_vp); fs = ip->i_fs; - if (ap->a_vp->v_type == VCHR) + if (ap->a_vp->v_type == VCHR || ap->a_vp->v_type == VBLK) return (EOPNOTSUPP); error = extattr_check_cred(ap->a_vp, ap->a_attrnamespace, @@ -1605,7 +1605,7 @@ vop_listextattr { ip = VTOI(ap->a_vp); fs = ip->i_fs; - if (ap->a_vp->v_type == VCHR) + if (ap->a_vp->v_type == VCHR || ap->a_vp->v_type == VBLK) return (EOPNOTSUPP); error = extattr_check_cred(ap->a_vp, ap->a_attrnamespace, @@ -1668,7 +1668,7 @@ vop_setextattr { ip = VTOI(ap->a_vp); fs = ip->i_fs; - if (ap->a_vp->v_type == VCHR) + if (ap->a_vp->v_type == VCHR || ap->a_vp->v_type == VBLK) return (EOPNOTSUPP); if (strlen(ap->a_name) == 0) _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Author: trasz Date: Thu Jul 2 20:05:21 2009 New Revision: 195296 URL: http://svn.freebsd.org/changeset/base/195296 Log: Fix fpathconf(3) on fifos, in effect making ls(1) properly display '+' on them. Taken from kern/125613, with cosmetic changes. PR: kern/125613 Submitted by: Jaakko Heinonen <jh at saunalahti dot fi> Approved by: re (kib) Modified: head/sys/ufs/ufs/ufs_vnops.c head/tools/regression/acltools/tools-posix.test Modified: head/sys/ufs/ufs/ufs_vnops.c ============================================================================== --- head/sys/ufs/ufs/ufs_vnops.c Thu Jul 2 18:24:37 2009 (r195295) +++ head/sys/ufs/ufs/ufs_vnops.c Thu Jul 2 20:05:21 2009 (r195296) @@ -112,6 +112,7 @@ static vop_symlink_t ufs_symlink; static vop_whiteout_t ufs_whiteout; static vop_close_t ufsfifo_close; static vop_kqfilter_t ufsfifo_kqfilter; +static vop_pathconf_t ufsfifo_pathconf; /* * A virgin directory (no blushing please). @@ -2101,6 +2102,29 @@ ufsfifo_kqfilter(ap) } /* + * Return POSIX pathconf information applicable to fifos. + */ +static int +ufsfifo_pathconf(ap) + struct vop_pathconf_args /* { + struct vnode *a_vp; + int a_name; + int *a_retval; + } */ *ap; +{ + + switch (ap->a_name) { + case _PC_ACL_EXTENDED: + case _PC_ACL_PATH_MAX: + case _PC_MAC_PRESENT: + return (ufs_pathconf(ap)); + default: + return (fifo_specops.vop_pathconf(ap)); + } + /* NOTREACHED */ +} + +/* * Return POSIX pathconf information applicable to ufs filesystems. */ static int @@ -2520,6 +2544,7 @@ struct vop_vector ufs_fifoops = { .vop_inactive = ufs_inactive, .vop_kqfilter = ufsfifo_kqfilter, .vop_markatime = ufs_markatime, + .vop_pathconf = ufsfifo_pathconf, .vop_print = ufs_print, .vop_read = VOP_PANIC, .vop_reclaim = ufs_reclaim, Modified: head/tools/regression/acltools/tools-posix.test ============================================================================== --- head/tools/regression/acltools/tools-posix.test Thu Jul 2 18:24:37 2009 (r195295) +++ head/tools/regression/acltools/tools-posix.test Thu Jul 2 20:05:21 2009 (r195296) @@ -353,3 +353,37 @@ $ rmdir ddd/ddd $ rm ddd/xxx $ rmdir ddd +# Test if we deal properly with fifos. +$ mkfifo fff +$ ls -l fff | cut -d' ' -f1 +> prw-r--r-- + +$ setfacl -m u:42:r,g:43:w fff +$ getfacl fff +> # file: fff +> # owner: root +> # group: wheel +> user::rw- +> user:42:r-- +> group::r-- +> group:43:-w- +> mask::rw- +> other::r-- + +$ ls -l fff | cut -d' ' -f1 +> prw-rw-r--+ + +$ setfacl -bn fff +$ getfacl fff +> # file: fff +> # owner: root +> # group: wheel +> user::rw- +> group::r-- +> other::r-- + +$ ls -l fff | cut -d' ' -f1 +> prw-r--r-- + +$ rm fff + _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Responsible Changed From-To: rwatson->trasz I'll take it.
Closing bug that seems to have been fixed on all shipping branches. Thanks trasz :)!