Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 29 May 2006 10:00:41 GMT
From:      Bruce Evans <bde@zeta.org.au>
To:        freebsd-bugs@FreeBSD.org
Subject:   Re: kern/98064: Crash with FIFOs (named pipes) and truncate()
Message-ID:  <200605291000.k4TA0fxK092979@freefall.freebsd.org>

next in thread | raw e-mail | index | archive | help
The following reply was made to PR kern/98064; it has been noted by GNATS.

From: Bruce Evans <bde@zeta.org.au>
To: Kirk Russell <kirk@ba23.org>
Cc: freebsd-gnats-submit@freebsd.org
Subject: Re: kern/98064: Crash with FIFOs (named pipes) and truncate()
Date: Mon, 29 May 2006 19:53:12 +1000 (EST)

 On Sun, 28 May 2006, Kirk Russell wrote:
 
 [... lots, but I inadvertently deleted the mail that I meant to reply to]
 
 I have used used the following fixes in this area for many years.  They
 make truncate() on a fifo and some other file types always succeed
 instead of wandering off into UFS_TRUNCATE() (which is always (?)
 ffs_truncate()) and tending to cause panics there.
 
 % Index: ufs_vnops.c
 % ===================================================================
 % RCS file: /home/ncvs/src/sys/ufs/ufs/ufs_vnops.c,v
 % retrieving revision 1.239
 % diff -u -2 -r1.239 ufs_vnops.c
 % --- ufs_vnops.c	7 Apr 2004 03:47:20 -0000	1.239
 % +++ ufs_vnops.c	15 Feb 2005 09:56:50 -0000
 % @@ -542,7 +573,6 @@
 %  	if (vap->va_size != VNOVAL) {
 %  		/*
 % -		 * Disallow write attempts on read-only filesystems;
 % -		 * unless the file is a socket, fifo, or a block or
 % -		 * character device resident on the filesystem.
 % +		 * XXX most of this checking should be in callers instead
 % +		 * of in N filesystems.  The VDIR check mostly already is.
 %  		 */
 %  		switch (vp->v_type) {
 
 The comment didn't even echo the code, since writes are now also disallowed
 for snapshots and block devices aren't supported.  I moved it and expanded
 it (except for not trying to list the file types in the "unless" case),
 though I think comments that echo code are negatively useful.
 
 % @@ -551,4 +581,15 @@
 %  		case VLNK:
 %  		case VREG:
 % +			/*
 % +			 * Truncation should have an effect in these cases.
 % +			 * Disallow it if the filesystem is read-only or
 % +			 * the file is being snapshotted.
 % +			 *
 % +			 * XXX unfortunately the snapshot check can't be
 % +			 * more global since we want to check other things
 % +			 * first so as to return better error codes.  But
 % +			 * we miss several cases (file flags and ownership
 % +			 * changes at least) by not doing a central check.
 
 After writing this I learned that allowing changing the attributes of
 snapshots for the "missed" cases is intentional.  I haven't seen any
 documentation or rationale of what can be changed.  The strangest
 restrictions are that certain chmod()s are not allowed and all utimes()
 are not allowed.  All chown()s are allowed, so the ctimes of snapshots
 can be changed to the current time using a null chown().
 
 % +			 */
 %  			if (vp->v_mount->mnt_flag & MNT_RDONLY)
 %  				return (EROFS);
 % @@ -557,5 +598,17 @@
 %  			break;
 %  		default:
 % -			break;
 % +			/*
 % +			 * According to POSIX, the result is unspecified
 % +			 * for file types other than regular files,
 % +			 * directories and shared memory objects.  We
 % +			 * don't support shared memory objects in the file
 % +			 * system, and have dubious support for truncating
 % +			 * symlinks (I think it implements panic(3) if
 % +			 * DIAGNOSTIC is enabled.)  Just ignore the request
 % +			 * in other cases.
 
 The support for the VLNK case is bogus.  ffs_truncate() has a fair amount
 of code to support this case, but if DIAGNOSTIC is configured the
 ffs_truncate() just panics for symlinks.  This patch doesn't change that.
 However, I think the VLNK case is unreachable even here.  truncate(2)
 follows links so it is impossible to truncate(2) a symlink, and open(2)
 follows symlinks so it is impossible to ftruncate(2) a symlink.  So only
 the kernel could call here for a symlink, and I think it doesn't.
 
 % +			 *
 % +			 * XXX update the man page.
 % +			 */
 % +			return (0);
 
 This return makes truncate() on a fifo do nothing.
 
 %  		}
 % if ((error = UFS_TRUNCATE(vp, vap->va_size, IO_NORMAL,
 
 All this should be handled using vnops in individual file systems and no
 comments here.
 
 Bruce



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200605291000.k4TA0fxK092979>