Re: Something badly broken with the latest XFS changeset in all stable kernels?

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Jun 13, 2016 at 11:57:53PM +0200, Thomas D. wrote:
> Hi,
> 
> can anybody confirm if there's something broken with the latest XFS
> change set which is now applied on all stable kernels?
> 
> I found https://forums.grsecurity.net/viewtopic.php?t=4489&p=16355 and
> grsec changelog says
> 
> > commit 1f621dc42acbabb71bd69f6ba606cee56e7ad3bc
> > Author: Brad Spengler <spender@xxxxxxxxxxxxxx>
> > Date:   Sat Jun 11 08:14:32 2016 -0400
> > 
> >     Fix Greg KH's broken XFS backport, caused a benign case to be detected
> >     as disk corruption
> >     Problem was due to a tree-wide conversion of error codes to their negative
> >     counterparts, which would likely never be backported to older kernels, but
> >     the backports didn't account for the change
> > 
> >  fs/xfs/xfs_inode.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)

Yes, the backport is busted, and needs this fix. The error sign
change occurred in 3.17. xfstests would have picked up this
regression in a couple of minutes, so I'm guessing that none of
these stable releases have had any significant regression testing
done....

> This is the change grsec applied:
> 
> > diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> > index fb8579d..af807d8 100644
> > --- a/fs/xfs/xfs_inode.c
> > +++ b/fs/xfs/xfs_inode.c
> > @@ -3098,7 +3111,7 @@ xfs_iflush(
> >  	 */
> >  	error = xfs_imap_to_bp(mp, NULL, &ip->i_imap, &dip, &bp, XBF_TRYLOCK,
> >  			       0);
> > -	if (error == -EAGAIN) {
> > +	if (error == EAGAIN) {
> >  		xfs_ifunlock(ip);
> >  		return error;
> >  	}

Yes, that is the fix that is needed. Thank you for reporting it to
us.

Mr Spender: it would be appreciated if you reported stable kernel
regressions to the relevant upstream maintainers so they can be
fixed quickly for everyone, rather than having one of your users
decide it needs to be reported.

Stable kernel maintainers: the above error sign change is needed for
stable kernels 3.16 and earlier, as a matter of critical importance.
And as a further matter of critical importance: in future, please
take the time to regression test the changes you backport.

> The bad commit according to grsec's statement is
> 
> > From b1438f477934f5a4d5a44df26f3079a7575d5946 Mon Sep 17 00:00:00 2001
> > From: Dave Chinner <dchinner@xxxxxxxxxx>
> > Date: Wed, 18 May 2016 13:53:42 +1000
> > Subject: [PATCH] xfs: xfs_iflush_cluster fails to abort on error
> 
> Would be nice to get some clarification.

There's nothing wrong with that commit in the upstream kernel,
it's the backport that has a bug in it because it failed to take
into account changes outside the context of the upstream commit that
the older kernels don't have.

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs



[Index of Archives]     [Linux XFS Devel]     [Linux Filesystem Development]     [Filesystem Testing]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux