On Wed, Apr 12, 2017 at 7:45 PM, Darrick J. Wong <darrick.wong@xxxxxxxxxx> wrote: > 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@xxxxxxxxxx> >> > > >> > > 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. > Ok, I will send again only the dump patch with modified message (+ man page update), without this mdrestore patch. That way it should pass and meanwhile, we can continue here about what to do (if anything) with mdrestore. Jan -- Jan Tulak jtulak@xxxxxxxxxx / jan@xxxxxxxx -- 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
