Re: [PATCH 2/2] mdrestore: warn about corruption if log is dirty

["Darrick J. Wong" <darrick.wong@oracle.com>]

On Wed, Apr 12, 2017 at 07:06:33AM -0400, Brian Foster wrote:
> On Tue, Apr 11, 2017 at 04:43:26PM -0700, Darrick J. Wong wrote:
> > On Wed, Apr 12, 2017 at 08:34:05AM +1000, Dave Chinner wrote:
> > > On Tue, Apr 11, 2017 at 04:12:37PM +0200, Jan Tulak wrote:
> > > > A dirty log in an obfuscated dump means that a corruption can happen
> > > > when replaying the log (which contains unobfuscated data). Warn the user
> > > > about this possibility.
> > >
> > > > The xlog workaround is copy&paste solution from repair/phase2.c and
> > > > other tools, because the function is not implemented in libxlog.
> > > > 
> > > > Signed-off-by: Jan Tulak <jtulak@redhat.com>
> > > 
> > > I think this is overkill. mdrestore is not the place
> > > to be interpreting the state of the dumped image - it is a basic
> > > "restore the image" program, not a "check the validity of the image"
> > > program.
> > >
> > > Secondly, if people are having problems with running log recovery on
> > > a restored obfuscated image and getting corruption and not knowing
> > > why or what to do, then that is a /documentation and training/
> > > problem, not a code problem.
> > >
> > > i.e. the problem is that people who aren't developers are trying to
> > > use tools that were written for developers to do forensic analysis
> > > of failures. Don't dumb down the tool for clueless users - point the
> > > users at the documentation that the tool requires to use correctly...
> > 
> > Looking at the patch, that's a lot of code to add to mdrestore that has
> > nothing to do with metadump restoration.  For that matter, who's to say
> > that the metadump'd image is even an XFS filesystem, and not just some
> > garbage with the just the right superblock values to pass the
> > perform_restore() checks?  (Ok, ok, that was a little over the top.)
> > 
> 
> Agreed wrt to the mdrestore bits...
> 
> > The key change we're trying to make is to prevent people incorrectly
> > replaying an XFS with a dirty log when the fs image has been restored
> > from an obfuscated metadump.
> > 
> > So in my mind this brings up two questions:  First, how do we prevent
> > log replay in such situations?  Second, how do we teach people not to
> > attempt log replay?  As you point out, it's better that we educate
> > people as what problems each tool tries to solve and where the sharp
> > edges might be on the debugging tools, but the answer to the first
> > question ensures that us fallible developers can't do something stupid
> > even though we theoretically know better.
> > 
> > Frankly, if the goal is to nudge n00b members of support teams away from
> > a behavior that won't help them towards starting their failure analysis,
> > then then I think we ought to patch the log recovery code to detect an
> > obfuscated fs image, complain to dmesg about someone making an illogical
> > move, and then refuse to mount the log.
> > 
> 
> I don't think this is really appropriate. Some users may very well have
> no other option but to create a dirty log + obfuscated metadump for
> whatever security/privacy reasons they have. The purpose of warning in
> that case is to notify the user to either verify the resulting image
> shows whatever problems are exhibited by the original fs and no others,
> or to notify the developer that other corruption might exist and to
> ignore it as a side effect of the metadump process itself (provided it
> doesn't interfere with rca of the original problem). Refusing to run log
> recovery in such cases just gets in the way.
> 
> I'm not tied to having an mdrestore warning at all, but I'd much prefer
> to see it there rather than include obfuscation logic in the kernel just
> to facilitate a userspace tool to continue on silently corrupting
> filesystem images.

<nod> I've changed my mind overnight.  Now I agree that we could put a
message in at metadump time, because it's not too late to ask the user
to try to send us a metadump w/ clean log.  Eric also convinced me that
it's not so trivial to detect an obfuscated image, so that simply won't
work without a bunch of hackery.

--D

> 
> Brian
> 
> > I'd rather push back on the incorrect behavior at the time it is done,
> > instead of training people to ignore a priori warning messages.
> > 
> > --D
> > 
> > > Cheers,
> > > 
> > > Dave.
> > > -- 
> > > Dave Chinner
> > > david@fromorbit.com
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> > > the body of a message to majordomo@vger.kernel.org
> > > 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@vger.kernel.org
> > 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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html