Re: [PATCH] fstests: test dirty pipe vulnerability issue of CVE-2022-0847

[Dave Chinner <david@fromorbit.com>]

On Wed, Mar 09, 2022 at 11:39:36AM +0800, Zorro Lang wrote:
> On Wed, Mar 09, 2022 at 09:48:58AM +1100, Dave Chinner wrote:
> > On Tue, Mar 08, 2022 at 11:55:01AM -0800, Darrick J. Wong wrote:
> > > On Wed, Mar 09, 2022 at 03:02:19AM +0800, Zorro Lang wrote:
> > > > On Tue, Mar 08, 2022 at 09:14:29AM -0800, Darrick J. Wong wrote:
> > > > > On Tue, Mar 08, 2022 at 05:22:48PM +0800, Zorro Lang wrote:
> > > > > > +chmod 0644 $localfile
> > > > > > +# Test privileged user (xfstests generally run with root)
> > > > > > +echo "Test privileged user:"
> > > > > > +$here/src/splice2pipe $localfile 1 "AAAAAAAABBBBBBBB"
> > > > > > +# Part of 0xff will be overwritten if there's CVE-2022-0847 bug
> > > > > > +hexdump -C $localfile
> > > > > 
> > > > > (I wonder offhand if fstests ought to be checking for the existence of
> > > > > hexdump(1) since at least Debian only has it in bsdmainutils package,
> > > > > but ... that's a separate question.)
> > > > 
> > > > Hmm... I never thought about that, due to the hexdump is in util-linux for
> > > > rhel and fedora. That means it's nearly always be there. If Debian or some other
> > > > system won't have it by default, we might say 'hexdump' is a necessary dependence
> > > > to run fstests in doc :)
> > > 
> > > Yeah.  The fstests documentation don't list bsdmainutils as a required
> > > package for Debian, so I think either we should update the documentation
> > > or do a treewide change to make all the tests that use hexdump(1)
> > > _require it.
> > 
> > I think it would be better to replace hexdump uses with 'od -x' as
> > od is part of coreutils. Hence we can either replace all the calls
> > to hexdump with direct calls to od -x, or add a simple wrapper like:
> > 
> > hexdump() {
> > 	od -x $@
> > }
> > 
> > with whatever the format specification needed is to output the same
> > format as hexdump does...
> 
> You're right, "od -x" output foramt is same with "hexdump" [1]. But most
> xfstests' cases use "hexdump -C", and output as golden image. I tried to make
> od output as that format, but didn't find a proper way. (Feel free to tell me
> if you know a better usage).

$ hexdump -C foo
00000000  73 61 64 6c 66 6b 6a 61  73 64 6c 66 6b 6a 75 69  |sadlfkjasdlfkjui|
00000010  79 72 75 69 79 6a 68 76  6b 76 6e 68 6b 6a 68 64  |yruiyjhvkvnhkjhd|
00000020  66 67 75 69 79 72 74 0a                           |fguiyrt.|
00000028
$ od  -Ax -t x1z -v foo
000000 73 61 64 6c 66 6b 6a 61 73 64 6c 66 6b 6a 75 69  >sadlfkjasdlfkjui<
000010 79 72 75 69 79 6a 68 76 6b 76 6e 68 6b 6a 68 64  >yruiyjhvkvnhkjhd<
000020 66 67 75 69 79 72 74 0a                          >fguiyrt.<
000028

Now, this is still not quite a perfect match - there's whitespace difference
and the char output at the end uses ">....<" delimiters rather than "|...|".
Hence I think that output file changes will be needed, but otherwise the
information being dropped in the output files is identical.

That said, the complexity of the 'od' command line means we really
need _hexdump() wrapper function would be a good idea. Maybe also:

hexdump()
{
	_fail "Use _hexdump(), please!"
}

might also be useful in preventing hexdump usage getting back into
fstests....

> So if we'd like to replace hexdump with od, we
> need to change some cases' .out files. Do you think it's time to do this change
> now? I can send another patch to do that.

When you've got time a new patch to do this would be great!

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com