[Top] [All Lists]

Re: bug: truncate to zero + setuid

To: Roger Willcocks <roger@xxxxxxxxxxxxxxxx>
Subject: Re: bug: truncate to zero + setuid
From: Tim Shimmin <tes@xxxxxxx>
Date: Mon, 29 Oct 2007 11:54:58 +1100
Cc: xfs@xxxxxxxxxxx
In-reply-to: <47249E7A.7060709@filmlight.ltd.uk>
References: <47249E7A.7060709@filmlight.ltd.uk>
Sender: xfs-bounce@xxxxxxxxxxx
User-agent: Thunderbird (Windows/20070809)
Hi Roger,

Roger Willcocks wrote:
The nfsv3 setattr call permits a simultaneous truncate + setuid/gid operation. Normally XFS handles this fine, but if the file's being truncated to zero, and the file's already empty, XFS simply ignores the setuid/gid part, returning 'success'.

The error's in xfs_vnodeops.c/xfs_setattr below the comment 'Short circuit the truncate case for zero length files', which bypasses all other changes.

The simplest fix is to test whether this is the only change that's happening, otherwise you get tangled in transactions.

       if (mask & XFS_AT_SIZE) {
               /* Short circuit the truncate case for zero length files */
-               if ((vap->va_size == 0) &&
+               if (((mask & ~XFS_AT_SIZE) == 0) && (vap->va_size == 0) &&
                  (ip->i_d.di_size == 0) && (ip->i_d.di_nextents == 0)) {

-- Roger

Yeah, I see your problem. It is interesting to know how much of multiple actions (different bits of mask) are supported. XFS_AT_UPDTIMES doesn't support anything else and will return quickly. As far as code which goes via error_return, it looks like the only non-error case is the short-circuit one mentioned above - truncating to zero an already zeroed file.

I see your fix will get what we want although, it will mean that we
will go thru the truncating path when we don't need to.
It would be nice if we could act like the XFS_AT_SIZE was never set
on the call in the case that other bits are set.
Though when we first test the AT_SIZE, we don't have the inode locked.
And I presume in that case we don't necessarily need to send a
DMAPI truncate event?
So I wonder if before the 1st AT_SIZE test that we have now,
in the AT_SIZE case and with other bits set,
we could lock the inode and test
for the zero truncate of zero-len file and if so then remove the AT_SIZE bit
and then go on to the other testing as if AT_SIZE was never set.
Hmmm, may be that just complicates things too much.


<Prev in Thread] Current Thread [Next in Thread>