This message generated a parse failure. Raw output follows here. Please use 'back' to navigate. From devnull@lkml.org Tue May 14 17:04:17 2024 Envelope-to: j@jasper.es Delivery-date: Fri, 24 May 2013 10:07:57 +0200 Received: from localhost ([127.0.0.1] helo=squeeze.vs19.net) by squeeze.vs19.net with esmtp (Exim 4.72) (envelope-from ) id 1Ufn2b-0000uy-JJ for j@jasper.es; Fri, 24 May 2013 10:07:57 +0200 Original-Recipient: rfc822;jasper@telfort.nl Received: from pop3.telfort.nl [213.75.3.52] by squeeze.vs19.net with POP3 (fetchmail-6.3.18) for (single-drop); Fri, 24 May 2013 10:07:57 +0200 (CEST) Received: from cpxmta-msg03.kpnxchange.com (10.94.114.24) by cpxmbs-msg01.support.local (8.6.060.17) id 516DA660006F941F for jasper@telfort.nl; Fri, 24 May 2013 10:04:25 +0200 Received: from cpsmtpb-ews02.kpnxchange.com (213.75.39.5) by cpxmta-msg03.kpnxchange.com (8.6.060.14) id 5192C39C002D4940 for jasper@telfort.nl; Fri, 24 May 2013 10:04:25 +0200 Received: from cpsps-ews18.kpnxchange.com ([10.94.84.184]) by cpsmtpb-ews02.kpnxchange.com with Microsoft SMTPSVC(7.5.7601.17514); Fri, 24 May 2013 10:04:02 +0200 Received: from vger.kernel.org ([209.132.180.67]) by cpsps-ews18.kpnxchange.com with Microsoft SMTPSVC(7.5.7601.17514); Fri, 24 May 2013 10:04:02 +0200 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1760099Ab3EXIDn (ORCPT ); Fri, 24 May 2013 04:03:43 -0400 Received: from ipmail06.adl2.internode.on.net ([150.101.137.129]:51724 "EHLO ipmail06.adl2.internode.on.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1760085Ab3EXIDd convert rfc822-to-8bit (ORCPT ); Fri, 24 Ma X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: ArkNAFsen1F5LEc//2dsb2JhbABZgwi9LYUgBAGBABd0giMBAQQBJxMcKAsIAxQBAwklDwUlAyEBEogHBboKFo1qaigSg1QDlzqRQYMhKg Received: from ppp121-44-71-63.lns20.syd6.internode.on.net (HELO dastard) ([121.44.71.63]) by ipmail06.adl2.internode.on.net with ESMTP; 24 May 2013 17:33:02 +0930 Received: from dave by dastard with local (Exim 4.76) (envelope-from ) id 1Ufmxo-0001I0-8L; Fri, 24 May 2013 18:03:00 +1000 Date: Fri, 24 May 2013 18:03:00 +1000 From: Dave Chinner To: Dave Jones , Linux Kernel , xfs@oss.sgi.com Subject: Re: XFS assertion from truncate. (3.10-rc2) Message-Id: <20130524080300.GI24543@dastard> References: <20130522051243.GH29466@dastard> <20130522052938.GA2573@redhat.com> <20130522055147.GI29466@dastard> <20130522215454.GL29466@dastard> <20130523184948.GA11151@redhat.com> <20130523223038.GB24543@dastard> <20130524004906.GA21171@redhat.com> <20130524012625 Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: 8BIT In-Reply-To: <20130524030300.GA30760@redhat.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-Id: X-Mailing-List: linux-kernel@vger.kernel.org X-OriginalArrivalTime: 24 May 2013 08:04:02.0102 (UTC) FILETIME=[40AF5560:01CE5855] X-RcptDomain: telfort.nl On Thu, May 23, 2013 at 11:03:00PM -0400, Dave Jones wrote: > On Thu, May 23, 2013 at 09:52:19PM -0400, Dave Jones wrote: > > On Fri, May 24, 2013 at 11:26:25AM +1000, Dave Chinner wrote: > > > > > You want to print the debug output if the masked value != 0. > > > > XFS (sda2): xfs_setattr_size: mask 0xaa69, masked 0x801 ii 0xffff8802129e98c0, d 0xffff88021757d970 path /davej/src/trinity/tmp/tmp.5/ > > ah, sneaky. There's some unprintable characters there that didn't > show up in my terminal when I cut-n-pasted. They made it to the > logs though.. > > XFS (sda2): xfs_setattr_size: mask 0xaa69, masked 0x801 ii 0xffff8802129e98c0, d 0xffff88021757d970 path /davej/src/trinity/tmp/tmp.5/ So the mask is: ATTR_OPEN | ATTR_FILE | ATTR_KILL_SUID | ATTR_FORCE | \ ATTR_CTIME | ATTR_MTIME | ATTR_MODE And so the mask is tripping the assert is (ATTR_KILL_SUID | ATTR_MODE). Ok, that now makes sense - that's consistent with open(O_TRUNCATE) on a suid file. How the hell has this gone unnoticed? > Hexdump: of that part.. 706d 352e 012f 0a01 > So filename is 0x01 0x01 > > Don't know if that's relevant to the bug or not.. No, it's not. It seems like the XFS code is simply broken, and has been for a couple of years. This is the commit that changed the code into the split we currently have now: commit c4ed4243c40f97ed5b7b121777bbbc6aeaa722f0 Author: Christoph Hellwig Date: Fri Jul 8 14:34:23 2011 +0200 xfs: split xfs_setattr Split up xfs_setattr into two functions, one for the complex truncate handling, and one for the trivial attribute updates. Also move both new routines to xfs_iops.c as they are fairly Linux-specific. Signed-off-by: Christoph Hellwig Reviewed-by: Alex Elder Reviewed-by: Dave Chinner Clearly there's a simple test that xfstests is missing, not to mention every other test suite out there that is run regularly on linux.... Awww, hell. xfstest generic/193 is *supposed* to test behaviour of suid/sgid bits and clearing them is various situations. You know what I'm about to say, don't you? The test doesn't test what it thinks it is testing. it puts the destination file in root directory of the xfstests harness, not in the filesystems being tested. So, on all my machines, it runs on ext3 filesystems, never on the ext4, btrfs, xfs, etc filesystems that I'm actually testing. That's beside the point, because it doesn't test truncate behaviour. But at least I know now why my attempts to reproduce the problem didn't work... Right, patch below should fix the problem. What a frustrating bug. Now, where's my bottle of scotch? Cheers, Dave. -- Dave Chinner david@fromorbit.com xfs: kill suid/sgid through the truncate path. From: Dave Chinner XFS has failed to kill suid/sgid bits correctly when truncating files of non-zero size since commit c4ed4243 ("xfs: split xfs_setattr") introduced in the 3.1 kernel. Fix it. Fix it. Signed-off-by: Dave Chinner --- fs/xfs/xfs_iops.c | 43 ++++++++++++++++++++++++++++--------------- 1 file changed, 28 insertions(+), 15 deletions(-) diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c index d82efaa..c255382 100644 --- a/fs/xfs/xfs_iops.c +++ b/fs/xfs/xfs_iops.c @@ -455,6 +455,24 @@ xfs_vn_getattr( return 0; } +static void +xfs_setattr_mode( + struct inode *inode, + struct iattr *iattr) +{ + struct xfs_inode *ip = XFS_I(inode); + umode_t mode = iattr->ia_mode; + + if (!in_group_p(inode->i_gid) && !capable(CAP_FSETID)) + mode &= ~S_ISGID; + + ip->i_d.di_mode &= S_IFMT; + ip->i_d.di_mode |= mode & ~S_IFMT; + + inode->i_mode &= S_IFMT; + inode->i_mode |= mode & ~S_IFMT; +} + int xfs_setattr_nonsize( struct xfs_inode *ip, @@ -606,18 +624,8 @@ xfs_setattr_nonsize( /* * Change file access modes. */ - if (mask & ATTR_MODE) { - umode_t mode = iattr->ia_mode; - - if (!in_group_p(inode->i_gid) && !capable(CAP_FSETID)) - mode &= ~S_ISGID; - - ip->i_d.di_mode &= S_IFMT; - ip->i_d.di_mode |= mode & ~S_IFMT; - - inode->i_mode &= S_IFMT; - inode->i_mode |= mode & ~S_IFMT; - } + if (mask & ATTR_MODE) + xfs_setattr_mode(inode, iattr); /* * Change file access or modified times. @@ -714,9 +722,8 @@ xfs_setattr_size( return XFS_ERROR(error); ASSERT(S_ISREG(ip->i_d.di_mode)); - ASSERT((mask & (ATTR_MODE|ATTR_UID|ATTR_GID|ATTR_ATIME|ATTR_ATIME_SET| - ATTR_MTIME_SET|ATTR_KILL_SUID|ATTR_KILL_SGID| - ATTR_KILL_PRIV|ATTR_TIMES_SET)) == 0); + ASSERT((mask & (ATTR_UID|ATTR_GID|ATTR_ATIME|ATTR_ATIME_SET| + ATTR_MTIME_SET|ATTR_KILL_PRIV|ATTR_TIMES_SET)) == 0); if (!(flags & XFS_ATTR_NOLOCK)) { lock_flags |= XFS_IOLOCK_EXCL; @@ -860,6 +867,12 @@ xfs_setattr_size( xfs_inode_clear_eofblocks_tag(ip); } + /* + * Change file access modes. + */ + if (mask & ATTR_MODE) + xfs_setattr_mode(inode, iattr); + if (mask & ATTR_CTIME) { inode->i_ctime = iattr->ia_ctime; ip->i_d.di_ctime.t_sec = iattr->ia_ctime.tv_sec; -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/