On Wed, Jan 04, 2017 at 12:36:24PM +0200, Amir Goldstein wrote: > On Tue, Jan 3, 2017 at 10:36 PM, Darrick J. Wong > <darrick.wong@xxxxxxxxxx> wrote: > > On Sun, Dec 25, 2016 at 08:09:34PM +0200, Amir Goldstein wrote: > >> Set all possible file type values for different types of files > >> and verify that xfs_repair detects the correct errors. > >> > >> When setting invalid file type values (e.g. core.mode = 0170644), > >> all files are expected to have been junked by xfs_repair. > >> > >> When setting valid file type values to non matching file types, > >> xfs_repair would either detect wrong format and junk the file, e.g.: > >> would have junked entry "FILE" in directory PARENT_INO > >> or detect a ftype mismatch error, e.g.: > >> would fix ftype mismatch (5/3) in directory/child PARENT_INO/FIFO_INO > >> > >> If ftype feature is enabled, when setting file type to one of the > >> special types (i.e. FIFO(1), CHRDEV(2),BLKDEV(6),SOCKET(14)), > >> xfs_repair is expected to detect ftype mismatch error. Otherewise, > > > > "Otherwise" > > > > thanks > > > I would also like to see a second test that scrambles the ftype field in > > the directory entry (instead of changing the inode core.mode) but xfs_db > > can't write to dir3 blocks because it doesn't know how to set the dir > > block CRC. > > > > I'm going to send a patch to add that as part of my xfsprogs 4.11 > > patchbomb (hopefully next week, but after the xfsprogs 4.9 release) so > > that test can wait. > > > > Sure, I'll beef up the test when that code arrives. > > >> + # If ftype feature is enabled, when setting file type to one of the > >> + # special types (i.e. FIFO(1), CHRDEV(2),BLKDEV(6),SOCKET(14)), > >> + # xfs_repair is expected to detect ftype mismatch error. Otherewise, > >> + # xfs_repair is not expected to detect ftype mismatch error. > >> + if [ "$FTYPE_FEATURE" = 1 ] && (echo ':1:2:6:14:' | grep -q ":$dt:"); then > >> + _scratch_xfs_repair -n 2>&1 | grep -q "^would fix ftype mismatch" || \ > >> + _fail "xfs_repair should fix ftype mismatch" > >> + else > >> + _scratch_xfs_repair -n 2>&1 | grep -q -v "^would fix ftype mismatch" || \ > >> + _fail "xfs_repair should not fix ftype mismatch" > > > > Just FYI the whole test will stop as soon as we hit a _fail. Please > > consider simply echoing a complaint to stdout so that the golden output > > diff will catch this, and we can see all the failing cases. > > > > Will do. > > Can you please reply to v2 patch series. > It contain 2 more patches beyond this one with additional testing of access > to mounted fs with the malformed inodes. > > My question is what to do WRT patch 3/3 which triggers an XFS Assert in the > kernel (readdir of a phony directory that is really a regular file or symlink). > http://www.spinics.net/lists/linux-xfs/msg03059.html > > Is this something of concern to you? Can you instruct me whether the assertion > should be fixed or propose a fix yourself? > Or is it just a non issue and we should not add patch 3/3 to this test. ASSERTs by definition halt the system, so it's only a crasher bug if CONFIG_XFS_DEBUG=y. However, your test writes garbage to the filesystem, so we must try to avoid interacting with corrupted junk when possible. I suggest bouncing corruption out to userspace by changing that ASSERT to: if (rval == 0 || args->dp->i_d.di_size == args->geo->blksize) return -EFSCORRUPTED; ...and perhaps following it up with a check in _dinode_verify to reject the inode if it has an obviously bogus i_size. --D > > Amir. > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
