kern/98064: Crash with FIFOs (named pipes) and truncate()
Bruce Evans
bde at zeta.org.au
Mon May 29 03:00:43 PDT 2006
The following reply was made to PR kern/98064; it has been noted by GNATS.
From: Bruce Evans <bde at zeta.org.au>
To: Kirk Russell <kirk at ba23.org>
Cc: freebsd-gnats-submit at 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
More information about the freebsd-bugs
mailing list