* [PATCH 0/2 v2] xfsprogs: metadump warns about dirty journal @ 2017-04-13 8:13 Jan Tulak 2017-04-13 8:13 ` [PATCH 1/2] metadump: warn about corruption if log is dirty Jan Tulak 2017-04-13 8:13 ` [PATCH 2/2] xfsprogs: update man for metadump about dirty log/obfuscation issue Jan Tulak 0 siblings, 2 replies; 21+ messages in thread From: Jan Tulak @ 2017-04-13 8:13 UTC (permalink / raw) To: linux-xfs; +Cc: Jan Tulak Hi guys Per the discussion in the previous patches, I'm sending this updated version. It does nothing with mdrestore, only adds a bit more elaborate warning on the dump side and adds the same info also to the man page for metadump. Hopefully, this is ok for everyone and we can continue the discussion if/what to do with mdrestore in the old thread. This set is based on xfsprogs: 07a3e79 for-next branch, git tree is https://github.com/jtulak/xfsprogs-dev/tree/metadump Cheers, Jan Jan Tulak (2): metadump: warn about corruption if log is dirty xfsprogs: update man for metadump about dirty log/obfuscation issue db/metadump.c | 4 +++- man/man8/xfs_metadump.8 | 6 ++++++ 2 files changed, 9 insertions(+), 1 deletion(-) -- 2.1.4 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 1/2] metadump: warn about corruption if log is dirty 2017-04-13 8:13 [PATCH 0/2 v2] xfsprogs: metadump warns about dirty journal Jan Tulak @ 2017-04-13 8:13 ` Jan Tulak 2017-04-13 11:54 ` Brian Foster 2017-04-13 8:13 ` [PATCH 2/2] xfsprogs: update man for metadump about dirty log/obfuscation issue Jan Tulak 1 sibling, 1 reply; 21+ messages in thread From: Jan Tulak @ 2017-04-13 8:13 UTC (permalink / raw) To: linux-xfs; +Cc: Jan Tulak Add a warning about possible corruption when exporting a dirty log, as the log content does not agree with obfuscated metadata. Signed-off-by: Jan Tulak <jtulak@redhat.com> --- Change: More elaborate warning message. --- db/metadump.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/db/metadump.c b/db/metadump.c index 66952f6..2dd8593 100644 --- a/db/metadump.c +++ b/db/metadump.c @@ -2726,7 +2726,9 @@ copy_log(void) /* keep the dirty log */ if (obfuscate) print_warning( -_("Filesystem log is dirty; image will contain unobfuscated metadata in log.")); +_("Warning: log recovery of an obfuscated metadata image can leak " +"unobfuscated metadata and/or cause image corruption. Please mount " +"the source filesystem to clean the log or disable obfuscation, if possible.")); break; case -1: /* log detection error */ -- 2.1.4 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] metadump: warn about corruption if log is dirty 2017-04-13 8:13 ` [PATCH 1/2] metadump: warn about corruption if log is dirty Jan Tulak @ 2017-04-13 11:54 ` Brian Foster 2017-06-15 0:06 ` Eric Sandeen 0 siblings, 1 reply; 21+ messages in thread From: Brian Foster @ 2017-04-13 11:54 UTC (permalink / raw) To: Jan Tulak; +Cc: linux-xfs On Thu, Apr 13, 2017 at 10:13:53AM +0200, Jan Tulak wrote: > Add a warning about possible corruption when exporting a dirty log, as > the log content does not agree with obfuscated metadata. > > Signed-off-by: Jan Tulak <jtulak@redhat.com> > --- > Change: More elaborate warning message. > --- > db/metadump.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/db/metadump.c b/db/metadump.c > index 66952f6..2dd8593 100644 > --- a/db/metadump.c > +++ b/db/metadump.c > @@ -2726,7 +2726,9 @@ copy_log(void) > /* keep the dirty log */ > if (obfuscate) > print_warning( > -_("Filesystem log is dirty; image will contain unobfuscated metadata in log.")); > +_("Warning: log recovery of an obfuscated metadata image can leak " > +"unobfuscated metadata and/or cause image corruption. Please mount " > +"the source filesystem to clean the log or disable obfuscation, if possible.")); > break; Thanks Jan, just one very minor nit having read this again... could we put the "if possible" closer to the part about mounting the source image? Otherwise it reads to me that it might not be technically possible to disable obfuscation, which is not the case (though the user may not want to do that as a matter of policy). For example: "... Please mount the source filesystem to clean the log, if possible, or disable obfuscation." With that tweak: Reviewed-by: Brian Foster <bfoster@redhat.com> > case -1: > /* log detection error */ > -- > 2.1.4 > > -- > 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] metadump: warn about corruption if log is dirty 2017-04-13 11:54 ` Brian Foster @ 2017-06-15 0:06 ` Eric Sandeen 2017-06-15 11:23 ` Brian Foster 0 siblings, 1 reply; 21+ messages in thread From: Eric Sandeen @ 2017-06-15 0:06 UTC (permalink / raw) To: Brian Foster, Jan Tulak; +Cc: linux-xfs On 4/13/17 6:54 AM, Brian Foster wrote: > On Thu, Apr 13, 2017 at 10:13:53AM +0200, Jan Tulak wrote: >> Add a warning about possible corruption when exporting a dirty log, as >> the log content does not agree with obfuscated metadata. >> >> Signed-off-by: Jan Tulak <jtulak@redhat.com> >> --- >> Change: More elaborate warning message. >> --- >> db/metadump.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/db/metadump.c b/db/metadump.c >> index 66952f6..2dd8593 100644 >> --- a/db/metadump.c >> +++ b/db/metadump.c >> @@ -2726,7 +2726,9 @@ copy_log(void) >> /* keep the dirty log */ >> if (obfuscate) >> print_warning( >> -_("Filesystem log is dirty; image will contain unobfuscated metadata in log.")); >> +_("Warning: log recovery of an obfuscated metadata image can leak " >> +"unobfuscated metadata and/or cause image corruption. Please mount " >> +"the source filesystem to clean the log or disable obfuscation, if possible.")); >> break; > > Thanks Jan, just one very minor nit having read this again... could we > put the "if possible" closer to the part about mounting the source > image? Otherwise it reads to me that it might not be technically > possible to disable obfuscation, which is not the case (though the user > may not want to do that as a matter of policy). For example: > > "... Please mount the source filesystem to clean the log, if possible, > or disable obfuscation." Hoovering up old patches ... To me, Jan's original is ok. "If possible" applying to both replay and disabling obfuscation seems reasonable, because "policy" may make it impossible ;) So I hate to direct the user to disable obfuscation. "If possible, please mount the filesystem to clean the log, or disable obfuscation." Is that OK? Gives the user options, and wiggle room.. -Eric > With that tweak: > > Reviewed-by: Brian Foster <bfoster@redhat.com> > >> case -1: >> /* log detection error */ >> -- >> 2.1.4 >> >> -- >> 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 > ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] metadump: warn about corruption if log is dirty 2017-06-15 0:06 ` Eric Sandeen @ 2017-06-15 11:23 ` Brian Foster 0 siblings, 0 replies; 21+ messages in thread From: Brian Foster @ 2017-06-15 11:23 UTC (permalink / raw) To: Eric Sandeen; +Cc: Jan Tulak, linux-xfs On Wed, Jun 14, 2017 at 07:06:52PM -0500, Eric Sandeen wrote: > On 4/13/17 6:54 AM, Brian Foster wrote: > > On Thu, Apr 13, 2017 at 10:13:53AM +0200, Jan Tulak wrote: > >> Add a warning about possible corruption when exporting a dirty log, as > >> the log content does not agree with obfuscated metadata. > >> > >> Signed-off-by: Jan Tulak <jtulak@redhat.com> > >> --- > >> Change: More elaborate warning message. > >> --- > >> db/metadump.c | 4 +++- > >> 1 file changed, 3 insertions(+), 1 deletion(-) > >> > >> diff --git a/db/metadump.c b/db/metadump.c > >> index 66952f6..2dd8593 100644 > >> --- a/db/metadump.c > >> +++ b/db/metadump.c > >> @@ -2726,7 +2726,9 @@ copy_log(void) > >> /* keep the dirty log */ > >> if (obfuscate) > >> print_warning( > >> -_("Filesystem log is dirty; image will contain unobfuscated metadata in log.")); > >> +_("Warning: log recovery of an obfuscated metadata image can leak " > >> +"unobfuscated metadata and/or cause image corruption. Please mount " > >> +"the source filesystem to clean the log or disable obfuscation, if possible.")); > >> break; > > > > Thanks Jan, just one very minor nit having read this again... could we > > put the "if possible" closer to the part about mounting the source > > image? Otherwise it reads to me that it might not be technically > > possible to disable obfuscation, which is not the case (though the user > > may not want to do that as a matter of policy). For example: > > > > "... Please mount the source filesystem to clean the log, if possible, > > or disable obfuscation." > > Hoovering up old patches ... > > To me, Jan's original is ok. "If possible" applying to both replay and > disabling obfuscation seems reasonable, because "policy" may make it > impossible ;) So I hate to direct the user to disable obfuscation. > > "If possible, please mount the filesystem to clean the log, or disable > obfuscation." > > Is that OK? Gives the user options, and wiggle room.. > Sounds good to me, thanks! Brian > -Eric > > > With that tweak: > > > > Reviewed-by: Brian Foster <bfoster@redhat.com> > > > >> case -1: > >> /* log detection error */ > >> -- > >> 2.1.4 > >> > >> -- > >> 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 2/2] xfsprogs: update man for metadump about dirty log/obfuscation issue 2017-04-13 8:13 [PATCH 0/2 v2] xfsprogs: metadump warns about dirty journal Jan Tulak 2017-04-13 8:13 ` [PATCH 1/2] metadump: warn about corruption if log is dirty Jan Tulak @ 2017-04-13 8:13 ` Jan Tulak 2017-04-13 12:01 ` Brian Foster 1 sibling, 1 reply; 21+ messages in thread From: Jan Tulak @ 2017-04-13 8:13 UTC (permalink / raw) To: linux-xfs; +Cc: Jan Tulak This is something that should be documented, as it is not obvious to everyone. Signed-off-by: Jan Tulak <jtulak@redhat.com> --- man/man8/xfs_metadump.8 | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/man/man8/xfs_metadump.8 b/man/man8/xfs_metadump.8 index 3731d6a..1b40fb8 100644 --- a/man/man8/xfs_metadump.8 +++ b/man/man8/xfs_metadump.8 @@ -59,6 +59,12 @@ options where are not obfuscated. Names between 5 and 8 characters in length inclusively are partially obfuscated. .PP +Log recovery of an obfuscated metadata image can leak +unobfuscated metadata and/or cause image corruption. Please mount +the source filesystem to clean the log or disable obfuscation, if possible. +If you have to obfuscate an image with a dirty log, tell about it to whoever +you are sending the image to. +.PP .B xfs_metadump should not be used for any purposes other than for debugging and reporting filesystem problems. The most common usage scenario for this tool is when -- 2.1.4 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] xfsprogs: update man for metadump about dirty log/obfuscation issue 2017-04-13 8:13 ` [PATCH 2/2] xfsprogs: update man for metadump about dirty log/obfuscation issue Jan Tulak @ 2017-04-13 12:01 ` Brian Foster 2017-04-13 12:29 ` Jan Tulak 0 siblings, 1 reply; 21+ messages in thread From: Brian Foster @ 2017-04-13 12:01 UTC (permalink / raw) To: Jan Tulak; +Cc: linux-xfs On Thu, Apr 13, 2017 at 10:13:54AM +0200, Jan Tulak wrote: > This is something that should be documented, as it is not obvious to > everyone. > > Signed-off-by: Jan Tulak <jtulak@redhat.com> > --- > man/man8/xfs_metadump.8 | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/man/man8/xfs_metadump.8 b/man/man8/xfs_metadump.8 > index 3731d6a..1b40fb8 100644 > --- a/man/man8/xfs_metadump.8 > +++ b/man/man8/xfs_metadump.8 > @@ -59,6 +59,12 @@ options where > are not obfuscated. Names between 5 and 8 characters in length inclusively > are partially obfuscated. > .PP > +Log recovery of an obfuscated metadata image can leak > +unobfuscated metadata and/or cause image corruption. Please mount > +the source filesystem to clean the log or disable obfuscation, if possible. > +If you have to obfuscate an image with a dirty log, tell about it to whoever > +you are sending the image to. > +.PP We might want the man page content to be a bit more descriptive than what xfs_metadump actually emits for a warning. For example: "xfs_metadump does not obfuscate data in the filesystem log. Log recovery of an obfuscated metadump image may expose unobfuscated metadata and/or cause filesystem corruption. It is recommended to disable obfuscation for filesystems that must be captured with a dirty log." ... but that's just my .02. Feel free to reword that and solicit more feedback from others too. Another thought here could be to intimate that if an obfuscated+dirty log metadump image is truly required, it is the user responsibility to verify that the resulting image has not been corrupted by the metadump process and does not contain sensitive metadata (as opposed to telling the user to simply tell the recipient of the image about it). Brian > .B xfs_metadump > should not be used for any purposes other than for debugging and reporting > filesystem problems. The most common usage scenario for this tool is when > -- > 2.1.4 > > -- > 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] xfsprogs: update man for metadump about dirty log/obfuscation issue 2017-04-13 12:01 ` Brian Foster @ 2017-04-13 12:29 ` Jan Tulak 2017-04-13 13:04 ` Brian Foster 2017-04-13 13:49 ` Eric Sandeen 0 siblings, 2 replies; 21+ messages in thread From: Jan Tulak @ 2017-04-13 12:29 UTC (permalink / raw) To: Brian Foster; +Cc: linux-xfs On Thu, Apr 13, 2017 at 2:01 PM, Brian Foster <bfoster@redhat.com> wrote: > On Thu, Apr 13, 2017 at 10:13:54AM +0200, Jan Tulak wrote: >> This is something that should be documented, as it is not obvious to >> everyone. >> >> Signed-off-by: Jan Tulak <jtulak@redhat.com> >> --- >> man/man8/xfs_metadump.8 | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/man/man8/xfs_metadump.8 b/man/man8/xfs_metadump.8 >> index 3731d6a..1b40fb8 100644 >> --- a/man/man8/xfs_metadump.8 >> +++ b/man/man8/xfs_metadump.8 >> @@ -59,6 +59,12 @@ options where >> are not obfuscated. Names between 5 and 8 characters in length inclusively >> are partially obfuscated. >> .PP >> +Log recovery of an obfuscated metadata image can leak >> +unobfuscated metadata and/or cause image corruption. Please mount >> +the source filesystem to clean the log or disable obfuscation, if possible. >> +If you have to obfuscate an image with a dirty log, tell about it to whoever >> +you are sending the image to. >> +.PP > > We might want the man page content to be a bit more descriptive than > what xfs_metadump actually emits for a warning. For example: > > "xfs_metadump does not obfuscate data in the filesystem log. Log > recovery of an obfuscated metadump image may expose unobfuscated > metadata and/or cause filesystem corruption. It is recommended to > disable obfuscation for filesystems that must be captured with a dirty > log." > > ... but that's just my .02. Feel free to reword that and solicit more > feedback from others too. Another thought here could be to intimate that > if an obfuscated+dirty log metadump image is truly required, it is the > user responsibility to verify that the resulting image has not been > corrupted by the metadump process and does not contain sensitive > metadata (as opposed to telling the user to simply tell the recipient of > the image about it). > Sounds reasonable, so how about these two paragraphs? > "xfs_metadump does not obfuscate data in the filesystem log. Log > recovery of an obfuscated metadump image may expose unobfuscated > metadata and/or cause filesystem corruption. It is recommended to > disable obfuscation for filesystems that must be captured with a dirty > log. If it is necessary to use obfuscation for any reason and the source fileystem can't be mounted to clean the log, the resulting image should be tested for any corruption caused by metadump process and any sensitive information in the log and the recipient of the image informed about the result." Cheers, Jan -- Jan Tulak jtulak@redhat.com / jan@tulak.me ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] xfsprogs: update man for metadump about dirty log/obfuscation issue 2017-04-13 12:29 ` Jan Tulak @ 2017-04-13 13:04 ` Brian Foster 2017-04-13 13:49 ` Eric Sandeen 1 sibling, 0 replies; 21+ messages in thread From: Brian Foster @ 2017-04-13 13:04 UTC (permalink / raw) To: Jan Tulak; +Cc: linux-xfs On Thu, Apr 13, 2017 at 02:29:34PM +0200, Jan Tulak wrote: > On Thu, Apr 13, 2017 at 2:01 PM, Brian Foster <bfoster@redhat.com> wrote: > > On Thu, Apr 13, 2017 at 10:13:54AM +0200, Jan Tulak wrote: > >> This is something that should be documented, as it is not obvious to > >> everyone. > >> > >> Signed-off-by: Jan Tulak <jtulak@redhat.com> > >> --- > >> man/man8/xfs_metadump.8 | 6 ++++++ > >> 1 file changed, 6 insertions(+) > >> > >> diff --git a/man/man8/xfs_metadump.8 b/man/man8/xfs_metadump.8 > >> index 3731d6a..1b40fb8 100644 > >> --- a/man/man8/xfs_metadump.8 > >> +++ b/man/man8/xfs_metadump.8 > >> @@ -59,6 +59,12 @@ options where > >> are not obfuscated. Names between 5 and 8 characters in length inclusively > >> are partially obfuscated. > >> .PP > >> +Log recovery of an obfuscated metadata image can leak > >> +unobfuscated metadata and/or cause image corruption. Please mount > >> +the source filesystem to clean the log or disable obfuscation, if possible. > >> +If you have to obfuscate an image with a dirty log, tell about it to whoever > >> +you are sending the image to. > >> +.PP > > > > We might want the man page content to be a bit more descriptive than > > what xfs_metadump actually emits for a warning. For example: > > > > "xfs_metadump does not obfuscate data in the filesystem log. Log > > recovery of an obfuscated metadump image may expose unobfuscated > > metadata and/or cause filesystem corruption. It is recommended to > > disable obfuscation for filesystems that must be captured with a dirty > > log." > > > > ... but that's just my .02. Feel free to reword that and solicit more > > feedback from others too. Another thought here could be to intimate that > > if an obfuscated+dirty log metadump image is truly required, it is the > > user responsibility to verify that the resulting image has not been > > corrupted by the metadump process and does not contain sensitive > > metadata (as opposed to telling the user to simply tell the recipient of > > the image about it). > > > > Sounds reasonable, so how about these two paragraphs? > > > "xfs_metadump does not obfuscate data in the filesystem log. Log > > recovery of an obfuscated metadump image may expose unobfuscated > > metadata and/or cause filesystem corruption. It is recommended to > > disable obfuscation for filesystems that must be captured with a dirty > > log. > > If it is necessary to use obfuscation for any reason and the source fileystem > can't be mounted to clean the log, the resulting image should be tested for > any corruption caused by metadump process and any sensitive information > in the log and the recipient of the image informed about the result." > You might want to give Eric/Darrick some time to provide feedback before sending out a new version, but otherwise that works for me. Thanks! Brian > Cheers, > Jan > > -- > Jan Tulak > jtulak@redhat.com / jan@tulak.me > -- > 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] xfsprogs: update man for metadump about dirty log/obfuscation issue 2017-04-13 12:29 ` Jan Tulak 2017-04-13 13:04 ` Brian Foster @ 2017-04-13 13:49 ` Eric Sandeen 2017-04-13 15:50 ` Darrick J. Wong 1 sibling, 1 reply; 21+ messages in thread From: Eric Sandeen @ 2017-04-13 13:49 UTC (permalink / raw) To: Jan Tulak, Brian Foster; +Cc: linux-xfs On 4/13/17 7:29 AM, Jan Tulak wrote: > On Thu, Apr 13, 2017 at 2:01 PM, Brian Foster <bfoster@redhat.com> wrote: >> On Thu, Apr 13, 2017 at 10:13:54AM +0200, Jan Tulak wrote: >>> This is something that should be documented, as it is not obvious to >>> everyone. >>> >>> Signed-off-by: Jan Tulak <jtulak@redhat.com> >>> --- >>> man/man8/xfs_metadump.8 | 6 ++++++ >>> 1 file changed, 6 insertions(+) >>> >>> diff --git a/man/man8/xfs_metadump.8 b/man/man8/xfs_metadump.8 >>> index 3731d6a..1b40fb8 100644 >>> --- a/man/man8/xfs_metadump.8 >>> +++ b/man/man8/xfs_metadump.8 >>> @@ -59,6 +59,12 @@ options where >>> are not obfuscated. Names between 5 and 8 characters in length inclusively >>> are partially obfuscated. >>> .PP >>> +Log recovery of an obfuscated metadata image can leak >>> +unobfuscated metadata and/or cause image corruption. Please mount >>> +the source filesystem to clean the log or disable obfuscation, if possible. >>> +If you have to obfuscate an image with a dirty log, tell about it to whoever >>> +you are sending the image to. >>> +.PP >> >> We might want the man page content to be a bit more descriptive than >> what xfs_metadump actually emits for a warning. For example: >> >> "xfs_metadump does not obfuscate data in the filesystem log. Log >> recovery of an obfuscated metadump image may expose unobfuscated >> metadata and/or cause filesystem corruption. It is recommended to >> disable obfuscation for filesystems that must be captured with a dirty >> log." >> >> ... but that's just my .02. Feel free to reword that and solicit more >> feedback from others too. Another thought here could be to intimate that >> if an obfuscated+dirty log metadump image is truly required, it is the >> user responsibility to verify that the resulting image has not been >> corrupted by the metadump process and does not contain sensitive >> metadata (as opposed to telling the user to simply tell the recipient of >> the image about it). >> > > Sounds reasonable, so how about these two paragraphs? > >> "xfs_metadump does not obfuscate data metadata >> in the filesystem log. Log >> recovery of an obfuscated metadump image may expose unobfuscated >> metadata and/or cause filesystem corruption. I would add "in the restored image." so that it does not sound like damage to the original filesystem could result. > It is recommended to >> disable obfuscation for filesystems that must be captured with a dirty >> log. > > If it is necessary to use obfuscation for any reason drop "for any reason" - just makes this long text longer... > and the source fileystem > can't be mounted to clean the log, the resulting image should be tested for > any corruption caused by metadump process and any sensitive information > in the log and the recipient of the image informed about the result." The end-user running metadump may not have the expertise to "test the resulting image for any corruption ..." - they're probably just following some support person's instructions to "run this command..." How about this: --- xfs_metadump cannot obfuscate metadata in the filesystem log. Log recovery of an obfuscated metadump image may expose clear-text metadata and/or cause filesystem corruption in the restored image. It is recommended that the source filesystem be mounted and unmounted first if possible, to produce a metadump with a clean log. If a metadump must be produced from a filesystem with a dirty log, it is recommended that obfuscation be turned off with -o option, if metadata such as filenames is not considered sensitive. If obfuscation is required on a metadump with a dirty log, please inform the recipient of the metadump image about this situation. --- -Eric ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] xfsprogs: update man for metadump about dirty log/obfuscation issue 2017-04-13 13:49 ` Eric Sandeen @ 2017-04-13 15:50 ` Darrick J. Wong 2017-04-13 17:01 ` Eric Sandeen 0 siblings, 1 reply; 21+ messages in thread From: Darrick J. Wong @ 2017-04-13 15:50 UTC (permalink / raw) To: Eric Sandeen; +Cc: Jan Tulak, Brian Foster, linux-xfs On Thu, Apr 13, 2017 at 08:49:52AM -0500, Eric Sandeen wrote: > On 4/13/17 7:29 AM, Jan Tulak wrote: > > On Thu, Apr 13, 2017 at 2:01 PM, Brian Foster <bfoster@redhat.com> wrote: > >> On Thu, Apr 13, 2017 at 10:13:54AM +0200, Jan Tulak wrote: > >>> This is something that should be documented, as it is not obvious to > >>> everyone. > >>> > >>> Signed-off-by: Jan Tulak <jtulak@redhat.com> > >>> --- > >>> man/man8/xfs_metadump.8 | 6 ++++++ > >>> 1 file changed, 6 insertions(+) > >>> > >>> diff --git a/man/man8/xfs_metadump.8 b/man/man8/xfs_metadump.8 > >>> index 3731d6a..1b40fb8 100644 > >>> --- a/man/man8/xfs_metadump.8 > >>> +++ b/man/man8/xfs_metadump.8 > >>> @@ -59,6 +59,12 @@ options where > >>> are not obfuscated. Names between 5 and 8 characters in length inclusively > >>> are partially obfuscated. > >>> .PP > >>> +Log recovery of an obfuscated metadata image can leak > >>> +unobfuscated metadata and/or cause image corruption. Please mount > >>> +the source filesystem to clean the log or disable obfuscation, if possible. > >>> +If you have to obfuscate an image with a dirty log, tell about it to whoever > >>> +you are sending the image to. > >>> +.PP > >> > >> We might want the man page content to be a bit more descriptive than > >> what xfs_metadump actually emits for a warning. For example: > >> > >> "xfs_metadump does not obfuscate data in the filesystem log. Log > >> recovery of an obfuscated metadump image may expose unobfuscated > >> metadata and/or cause filesystem corruption. It is recommended to > >> disable obfuscation for filesystems that must be captured with a dirty > >> log." > >> > >> ... but that's just my .02. Feel free to reword that and solicit more > >> feedback from others too. Another thought here could be to intimate that > >> if an obfuscated+dirty log metadump image is truly required, it is the > >> user responsibility to verify that the resulting image has not been > >> corrupted by the metadump process and does not contain sensitive > >> metadata (as opposed to telling the user to simply tell the recipient of > >> the image about it). > >> > > > > Sounds reasonable, so how about these two paragraphs? > > > >> "xfs_metadump does not obfuscate data > > metadata > > >> in the filesystem log. Log > >> recovery of an obfuscated metadump image may expose unobfuscated > >> metadata and/or cause filesystem corruption. > > I would add "in the restored image." so that it does not sound like > damage to the original filesystem could result. > > > It is recommended to > >> disable obfuscation for filesystems that must be captured with a dirty > >> log. > > > > If it is necessary to use obfuscation for any reason > > drop "for any reason" - just makes this long text longer... > > > and the source fileystem > > can't be mounted to clean the log, the resulting image should be tested for > > any corruption caused by metadump process and any sensitive information > > in the log and the recipient of the image informed about the result." > > The end-user running metadump may not have the expertise to "test the resulting > image for any corruption ..." - they're probably just following some support > person's instructions to "run this command..." How about this: (Or they're just running some crazy hoover-up-all-the-state script that support gave them. ;)) > --- > > xfs_metadump cannot obfuscate metadata in the filesystem log. Log > recovery of an obfuscated metadump image may expose clear-text > metadata and/or cause filesystem corruption in the restored image. > It is recommended that the source filesystem be mounted and unmounted > first if possible, to produce a metadump with a clean log. "It is recommended that the source filesystem first be mounted and unmounted, if possible, to ensure that the log is clean. That way, all metadata can be obfuscated correctly and the metadump will capture a clean log." ...because mounting and unmounting itself does not produce metadumps. :) > If a metadump must be produced from a filesystem with a dirty log, > it is recommended that obfuscation be turned off with -o option, if > metadata such as filenames is not considered sensitive. If obfuscation > is required on a metadump with a dirty log, please inform the recipient > of the metadump image about this situation. Otherwise seems fine to me. --D > --- > > -Eric > -- > 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] xfsprogs: update man for metadump about dirty log/obfuscation issue 2017-04-13 15:50 ` Darrick J. Wong @ 2017-04-13 17:01 ` Eric Sandeen 2017-04-13 17:06 ` Darrick J. Wong 0 siblings, 1 reply; 21+ messages in thread From: Eric Sandeen @ 2017-04-13 17:01 UTC (permalink / raw) To: Darrick J. Wong; +Cc: Jan Tulak, Brian Foster, linux-xfs On 4/13/17 10:50 AM, Darrick J. Wong wrote: > On Thu, Apr 13, 2017 at 08:49:52AM -0500, Eric Sandeen wrote: >> --- >> >> xfs_metadump cannot obfuscate metadata in the filesystem log. Log >> recovery of an obfuscated metadump image may expose clear-text >> metadata and/or cause filesystem corruption in the restored image. >> It is recommended that the source filesystem be mounted and unmounted >> first if possible, to produce a metadump with a clean log. > > "It is recommended that the source filesystem first be mounted and > unmounted, if possible, to ensure that the log is clean. That way, all > metadata can be obfuscated correctly and the metadump will capture a > clean log." > > ...because mounting and unmounting itself does not produce metadumps. :) That was the "first" bit, but "That way" is a bit colloquial IMHO. <full_pedant_mode> How about: "It is recommended that the source filesystem first be mounted and unmounted, if possible, to ensure that the log is clean. After that operation, all metadata will be obfuscated correctly and xfs_metadump will capture a clean log." -Eric >> If a metadump must be produced from a filesystem with a dirty log, >> it is recommended that obfuscation be turned off with -o option, if >> metadata such as filenames is not considered sensitive. If obfuscation >> is required on a metadump with a dirty log, please inform the recipient >> of the metadump image about this situation. > > Otherwise seems fine to me. > > --D > >> --- >> >> -Eric >> -- >> 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 > ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] xfsprogs: update man for metadump about dirty log/obfuscation issue 2017-04-13 17:01 ` Eric Sandeen @ 2017-04-13 17:06 ` Darrick J. Wong 0 siblings, 0 replies; 21+ messages in thread From: Darrick J. Wong @ 2017-04-13 17:06 UTC (permalink / raw) To: Eric Sandeen; +Cc: Jan Tulak, Brian Foster, linux-xfs On Thu, Apr 13, 2017 at 12:01:32PM -0500, Eric Sandeen wrote: > On 4/13/17 10:50 AM, Darrick J. Wong wrote: > > On Thu, Apr 13, 2017 at 08:49:52AM -0500, Eric Sandeen wrote: > > >> --- > >> > >> xfs_metadump cannot obfuscate metadata in the filesystem log. Log > >> recovery of an obfuscated metadump image may expose clear-text > >> metadata and/or cause filesystem corruption in the restored image. > >> It is recommended that the source filesystem be mounted and unmounted > >> first if possible, to produce a metadump with a clean log. > > > > "It is recommended that the source filesystem first be mounted and > > unmounted, if possible, to ensure that the log is clean. That way, all > > metadata can be obfuscated correctly and the metadump will capture a > > clean log." > > > > ...because mounting and unmounting itself does not produce metadumps. :) > > That was the "first" bit, but "That way" is a bit colloquial IMHO. > <full_pedant_mode> > > How about: > > "It is recommended that the source filesystem first be mounted and > unmounted, if possible, to ensure that the log is clean. After that > operation, all metadata will be obfuscated correctly and xfs_metadump > will capture a clean log." "A subsequent invocation of xfs_metadump will capture a clean log and obfuscate all metadata correctly." More bikeshedding! Yay! :) --D > > -Eric > > > >> If a metadump must be produced from a filesystem with a dirty log, > >> it is recommended that obfuscation be turned off with -o option, if > >> metadata such as filenames is not considered sensitive. If obfuscation > >> is required on a metadump with a dirty log, please inform the recipient > >> of the metadump image about this situation. > > > > Otherwise seems fine to me. > > > > --D > > > >> --- > >> > >> -Eric > >> -- > >> 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 0/2] xfsprogs: metadump/mdrestore warns about dirty journal @ 2017-04-11 14:12 Jan Tulak 2017-04-11 14:12 ` [PATCH 1/2] metadump: warn about corruption if log is dirty Jan Tulak 0 siblings, 1 reply; 21+ messages in thread From: Jan Tulak @ 2017-04-11 14:12 UTC (permalink / raw) To: linux-xfs; +Cc: sandeen, Jan Tulak If we are exporting obfuscated metadata with a dirty journal, the journal is not obfuscated. So when we restore it and then replay a log, the obfuscated metadata can clash with what is in journal and corrupt the image. So, tell the user about this risk, both when exporting and importing the dump. Git tree on github: https://github.com/jtulak/xfsprogs-dev/tree/metadump Based on revision fc1d645 Jan Tulak (2): metadump: warn about corruption if log is dirty mdrestore: warn about corruption if log is dirty db/metadump.c | 3 +- mdrestore/Makefile | 4 +-- mdrestore/xfs_mdrestore.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 88 insertions(+), 3 deletions(-) -- 2.1.4 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 1/2] metadump: warn about corruption if log is dirty 2017-04-11 14:12 [PATCH 0/2] xfsprogs: metadump/mdrestore warns about dirty journal Jan Tulak @ 2017-04-11 14:12 ` Jan Tulak 2017-04-11 18:30 ` Brian Foster 0 siblings, 1 reply; 21+ messages in thread From: Jan Tulak @ 2017-04-11 14:12 UTC (permalink / raw) To: linux-xfs; +Cc: sandeen, Jan Tulak Add a warning about possible corruption when exporting a dirty log, as the log content does not agree with obfuscated metadata. Signed-off-by: Jan Tulak <jtulak@redhat.com> --- db/metadump.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/db/metadump.c b/db/metadump.c index 66952f6..74e24b2 100644 --- a/db/metadump.c +++ b/db/metadump.c @@ -2726,7 +2726,8 @@ copy_log(void) /* keep the dirty log */ if (obfuscate) print_warning( -_("Filesystem log is dirty; image will contain unobfuscated metadata in log.")); +_("Filesystem log is dirty; image will contain unobfuscated metadata in log " + "and a log replay can cause a corruption.")); break; case -1: /* log detection error */ -- 2.1.4 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] metadump: warn about corruption if log is dirty 2017-04-11 14:12 ` [PATCH 1/2] metadump: warn about corruption if log is dirty Jan Tulak @ 2017-04-11 18:30 ` Brian Foster 2017-04-11 18:34 ` Eric Sandeen 0 siblings, 1 reply; 21+ messages in thread From: Brian Foster @ 2017-04-11 18:30 UTC (permalink / raw) To: Jan Tulak; +Cc: linux-xfs, sandeen On Tue, Apr 11, 2017 at 04:12:36PM +0200, Jan Tulak wrote: > Add a warning about possible corruption when exporting a dirty log, as > the log content does not agree with obfuscated metadata. > > Signed-off-by: Jan Tulak <jtulak@redhat.com> > --- Thanks for posting this... > db/metadump.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/db/metadump.c b/db/metadump.c > index 66952f6..74e24b2 100644 > --- a/db/metadump.c > +++ b/db/metadump.c > @@ -2726,7 +2726,8 @@ copy_log(void) > /* keep the dirty log */ > if (obfuscate) > print_warning( > -_("Filesystem log is dirty; image will contain unobfuscated metadata in log.")); > +_("Filesystem log is dirty; image will contain unobfuscated metadata in log " > + "and a log replay can cause a corruption.")); I think a slightly more verbose message might be a good idea. For example, something like the following: "Filesystem log is dirty; image will contain unobfuscated metadata in the log. Log recovery of an obfuscated image can cause filesystem corruption. Please mount the source image to clean the log or disable metadump obfuscation." That could also say "... or verify that log recovery of the resulting image does not cause corruption," but that might be overkill. Thoughts? Eric? Brian > break; > case -1: > /* log detection error */ > -- > 2.1.4 > > -- > 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] metadump: warn about corruption if log is dirty 2017-04-11 18:30 ` Brian Foster @ 2017-04-11 18:34 ` Eric Sandeen 2017-04-11 18:43 ` Brian Foster 0 siblings, 1 reply; 21+ messages in thread From: Eric Sandeen @ 2017-04-11 18:34 UTC (permalink / raw) To: Brian Foster, Jan Tulak; +Cc: linux-xfs On 4/11/17 1:30 PM, Brian Foster wrote: > On Tue, Apr 11, 2017 at 04:12:36PM +0200, Jan Tulak wrote: >> Add a warning about possible corruption when exporting a dirty log, as >> the log content does not agree with obfuscated metadata. >> >> Signed-off-by: Jan Tulak <jtulak@redhat.com> >> --- > > Thanks for posting this... > >> db/metadump.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/db/metadump.c b/db/metadump.c >> index 66952f6..74e24b2 100644 >> --- a/db/metadump.c >> +++ b/db/metadump.c >> @@ -2726,7 +2726,8 @@ copy_log(void) >> /* keep the dirty log */ >> if (obfuscate) >> print_warning( >> -_("Filesystem log is dirty; image will contain unobfuscated metadata in log.")); >> +_("Filesystem log is dirty; image will contain unobfuscated metadata in log " >> + "and a log replay can cause a corruption.")); > > I think a slightly more verbose message might be a good idea. For > example, something like the following: > > "Filesystem log is dirty; image will contain unobfuscated metadata in > the log. Log recovery of an obfuscated image can cause filesystem > corruption. Please mount the source image to clean the log or disable > metadump obfuscation." > > That could also say "... or verify that log recovery of the resulting > image does not cause corruption," but that might be overkill. Thoughts? > Eric? I think we do need a good explanation, but that will take a lot of workd. We could also refer to the man page for more details - it's getting pretty long for a warning from the tool. The other bummer is that we cannot detect whether an image has been obfuscated, so the restore tool can only say "if it's obfuscated...." -Eric > Brian > >> break; >> case -1: >> /* log detection error */ >> -- >> 2.1.4 >> >> -- >> 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 > ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] metadump: warn about corruption if log is dirty 2017-04-11 18:34 ` Eric Sandeen @ 2017-04-11 18:43 ` Brian Foster 2017-04-11 19:01 ` Eric Sandeen 0 siblings, 1 reply; 21+ messages in thread From: Brian Foster @ 2017-04-11 18:43 UTC (permalink / raw) To: Eric Sandeen; +Cc: Jan Tulak, linux-xfs On Tue, Apr 11, 2017 at 01:34:25PM -0500, Eric Sandeen wrote: > On 4/11/17 1:30 PM, Brian Foster wrote: > > On Tue, Apr 11, 2017 at 04:12:36PM +0200, Jan Tulak wrote: > >> Add a warning about possible corruption when exporting a dirty log, as > >> the log content does not agree with obfuscated metadata. > >> > >> Signed-off-by: Jan Tulak <jtulak@redhat.com> > >> --- > > > > Thanks for posting this... > > > >> db/metadump.c | 3 ++- > >> 1 file changed, 2 insertions(+), 1 deletion(-) > >> > >> diff --git a/db/metadump.c b/db/metadump.c > >> index 66952f6..74e24b2 100644 > >> --- a/db/metadump.c > >> +++ b/db/metadump.c > >> @@ -2726,7 +2726,8 @@ copy_log(void) > >> /* keep the dirty log */ > >> if (obfuscate) > >> print_warning( > >> -_("Filesystem log is dirty; image will contain unobfuscated metadata in log.")); > >> +_("Filesystem log is dirty; image will contain unobfuscated metadata in log " > >> + "and a log replay can cause a corruption.")); > > > > I think a slightly more verbose message might be a good idea. For > > example, something like the following: > > > > "Filesystem log is dirty; image will contain unobfuscated metadata in > > the log. Log recovery of an obfuscated image can cause filesystem > > corruption. Please mount the source image to clean the log or disable > > metadump obfuscation." > > > > That could also say "... or verify that log recovery of the resulting > > image does not cause corruption," but that might be overkill. Thoughts? > > Eric? > > I think we do need a good explanation, but that will take a lot of workd. > We could also refer to the man page for more details - it's getting pretty > long for a warning from the tool. > Hm, yeah. Maybe the existing warning can be condensed a bit more to something like: "Warning: log recovery of an obfuscated metadata image can leak unobfuscated metadata and/or cause filesystem corruption. Please mount the source image to clean the log or disable obfuscation." I suppose we could also just exit under such conditions unless the user passes a force flag or something. Maybe that's overkill too, though.. Brian > The other bummer is that we cannot detect whether an image has been > obfuscated, so the restore tool can only say "if it's obfuscated...." > > -Eric > > > Brian > > > >> break; > >> case -1: > >> /* log detection error */ > >> -- > >> 2.1.4 > >> > >> -- > >> 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] metadump: warn about corruption if log is dirty 2017-04-11 18:43 ` Brian Foster @ 2017-04-11 19:01 ` Eric Sandeen 2017-04-11 23:44 ` Darrick J. Wong 0 siblings, 1 reply; 21+ messages in thread From: Eric Sandeen @ 2017-04-11 19:01 UTC (permalink / raw) To: Brian Foster; +Cc: Jan Tulak, linux-xfs On 4/11/17 1:43 PM, Brian Foster wrote: > On Tue, Apr 11, 2017 at 01:34:25PM -0500, Eric Sandeen wrote: >> On 4/11/17 1:30 PM, Brian Foster wrote: >>> On Tue, Apr 11, 2017 at 04:12:36PM +0200, Jan Tulak wrote: >>>> Add a warning about possible corruption when exporting a dirty log, as >>>> the log content does not agree with obfuscated metadata. >>>> >>>> Signed-off-by: Jan Tulak <jtulak@redhat.com> >>>> --- >>> >>> Thanks for posting this... >>> >>>> db/metadump.c | 3 ++- >>>> 1 file changed, 2 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/db/metadump.c b/db/metadump.c >>>> index 66952f6..74e24b2 100644 >>>> --- a/db/metadump.c >>>> +++ b/db/metadump.c >>>> @@ -2726,7 +2726,8 @@ copy_log(void) >>>> /* keep the dirty log */ >>>> if (obfuscate) >>>> print_warning( >>>> -_("Filesystem log is dirty; image will contain unobfuscated metadata in log.")); >>>> +_("Filesystem log is dirty; image will contain unobfuscated metadata in log " >>>> + "and a log replay can cause a corruption.")); >>> >>> I think a slightly more verbose message might be a good idea. For >>> example, something like the following: >>> >>> "Filesystem log is dirty; image will contain unobfuscated metadata in >>> the log. Log recovery of an obfuscated image can cause filesystem >>> corruption. Please mount the source image to clean the log or disable >>> metadump obfuscation." >>> >>> That could also say "... or verify that log recovery of the resulting >>> image does not cause corruption," but that might be overkill. Thoughts? >>> Eric? >> >> I think we do need a good explanation, but that will take a lot of workd. >> We could also refer to the man page for more details - it's getting pretty >> long for a warning from the tool. >> > > Hm, yeah. Maybe the existing warning can be condensed a bit more to > something like: > > "Warning: log recovery of an obfuscated metadata image can leak > unobfuscated metadata and/or cause filesystem corruption. Please mount > the source image to clean the log or disable obfuscation." s/filesystem corruption/image corruption/ - we don't want anyone to think that it damaged the original fs! > I suppose we could also just exit under such conditions unless the user > passes a force flag or something. Maybe that's overkill too, though.. yeah, let's not overengineer - dumping a dirty, obfuscated log is quite typical I think. -Eric ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] metadump: warn about corruption if log is dirty 2017-04-11 19:01 ` Eric Sandeen @ 2017-04-11 23:44 ` Darrick J. Wong 2017-04-12 11:03 ` Brian Foster 0 siblings, 1 reply; 21+ messages in thread From: Darrick J. Wong @ 2017-04-11 23:44 UTC (permalink / raw) To: Eric Sandeen; +Cc: Brian Foster, Jan Tulak, linux-xfs On Tue, Apr 11, 2017 at 02:01:41PM -0500, Eric Sandeen wrote: > On 4/11/17 1:43 PM, Brian Foster wrote: > > On Tue, Apr 11, 2017 at 01:34:25PM -0500, Eric Sandeen wrote: > >> On 4/11/17 1:30 PM, Brian Foster wrote: > >>> On Tue, Apr 11, 2017 at 04:12:36PM +0200, Jan Tulak wrote: > >>>> Add a warning about possible corruption when exporting a dirty log, as > >>>> the log content does not agree with obfuscated metadata. > >>>> > >>>> Signed-off-by: Jan Tulak <jtulak@redhat.com> > >>>> --- > >>> > >>> Thanks for posting this... > >>> > >>>> db/metadump.c | 3 ++- > >>>> 1 file changed, 2 insertions(+), 1 deletion(-) > >>>> > >>>> diff --git a/db/metadump.c b/db/metadump.c > >>>> index 66952f6..74e24b2 100644 > >>>> --- a/db/metadump.c > >>>> +++ b/db/metadump.c > >>>> @@ -2726,7 +2726,8 @@ copy_log(void) > >>>> /* keep the dirty log */ > >>>> if (obfuscate) > >>>> print_warning( > >>>> -_("Filesystem log is dirty; image will contain unobfuscated metadata in log.")); > >>>> +_("Filesystem log is dirty; image will contain unobfuscated metadata in log " > >>>> + "and a log replay can cause a corruption.")); > >>> > >>> I think a slightly more verbose message might be a good idea. For > >>> example, something like the following: > >>> > >>> "Filesystem log is dirty; image will contain unobfuscated metadata in > >>> the log. Log recovery of an obfuscated image can cause filesystem > >>> corruption. Please mount the source image to clean the log or disable > >>> metadump obfuscation." > >>> > >>> That could also say "... or verify that log recovery of the resulting > >>> image does not cause corruption," but that might be overkill. Thoughts? > >>> Eric? > >> > >> I think we do need a good explanation, but that will take a lot of workd. > >> We could also refer to the man page for more details - it's getting pretty > >> long for a warning from the tool. > >> > > > > Hm, yeah. Maybe the existing warning can be condensed a bit more to > > something like: > > > > "Warning: log recovery of an obfuscated metadata image can leak > > unobfuscated metadata and/or cause filesystem corruption. Please mount > > the source image to clean the log or disable obfuscation." > > s/filesystem corruption/image corruption/ - we don't want anyone to think > that it damaged the original fs! OTOH the fs might be damaged just badly enough that log recovery is impossible (which is why we're creating the metadump to send to support) so aborting metadump is the wrong thing to do. It seems sort of silly even to lecture the user about log recovery when they might not be able to recover said log and might not ever even start the log recovery process. --D > > > I suppose we could also just exit under such conditions unless the user > > passes a force flag or something. Maybe that's overkill too, though.. > > yeah, let's not overengineer - dumping a dirty, obfuscated log is quite > typical I think. > > -Eric > -- > 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] metadump: warn about corruption if log is dirty 2017-04-11 23:44 ` Darrick J. Wong @ 2017-04-12 11:03 ` Brian Foster 2017-04-12 11:24 ` Jan Tulak 0 siblings, 1 reply; 21+ messages in thread From: Brian Foster @ 2017-04-12 11:03 UTC (permalink / raw) To: Darrick J. Wong; +Cc: Eric Sandeen, Jan Tulak, linux-xfs On Tue, Apr 11, 2017 at 04:44:44PM -0700, Darrick J. Wong wrote: > On Tue, Apr 11, 2017 at 02:01:41PM -0500, Eric Sandeen wrote: > > On 4/11/17 1:43 PM, Brian Foster wrote: > > > On Tue, Apr 11, 2017 at 01:34:25PM -0500, Eric Sandeen wrote: > > >> On 4/11/17 1:30 PM, Brian Foster wrote: > > >>> On Tue, Apr 11, 2017 at 04:12:36PM +0200, Jan Tulak wrote: > > >>>> Add a warning about possible corruption when exporting a dirty log, as > > >>>> the log content does not agree with obfuscated metadata. > > >>>> > > >>>> Signed-off-by: Jan Tulak <jtulak@redhat.com> > > >>>> --- > > >>> > > >>> Thanks for posting this... > > >>> > > >>>> db/metadump.c | 3 ++- > > >>>> 1 file changed, 2 insertions(+), 1 deletion(-) > > >>>> > > >>>> diff --git a/db/metadump.c b/db/metadump.c > > >>>> index 66952f6..74e24b2 100644 > > >>>> --- a/db/metadump.c > > >>>> +++ b/db/metadump.c > > >>>> @@ -2726,7 +2726,8 @@ copy_log(void) > > >>>> /* keep the dirty log */ > > >>>> if (obfuscate) > > >>>> print_warning( > > >>>> -_("Filesystem log is dirty; image will contain unobfuscated metadata in log.")); > > >>>> +_("Filesystem log is dirty; image will contain unobfuscated metadata in log " > > >>>> + "and a log replay can cause a corruption.")); > > >>> > > >>> I think a slightly more verbose message might be a good idea. For > > >>> example, something like the following: > > >>> > > >>> "Filesystem log is dirty; image will contain unobfuscated metadata in > > >>> the log. Log recovery of an obfuscated image can cause filesystem > > >>> corruption. Please mount the source image to clean the log or disable > > >>> metadump obfuscation." > > >>> > > >>> That could also say "... or verify that log recovery of the resulting > > >>> image does not cause corruption," but that might be overkill. Thoughts? > > >>> Eric? > > >> > > >> I think we do need a good explanation, but that will take a lot of workd. > > >> We could also refer to the man page for more details - it's getting pretty > > >> long for a warning from the tool. > > >> > > > > > > Hm, yeah. Maybe the existing warning can be condensed a bit more to > > > something like: > > > > > > "Warning: log recovery of an obfuscated metadata image can leak > > > unobfuscated metadata and/or cause filesystem corruption. Please mount > > > the source image to clean the log or disable obfuscation." > > > > s/filesystem corruption/image corruption/ - we don't want anyone to think > > that it damaged the original fs! > > OTOH the fs might be damaged just badly enough that log recovery is > impossible (which is why we're creating the metadump to send to support) > so aborting metadump is the wrong thing to do. > Indeed.. > It seems sort of silly even to lecture the user about log recovery when > they might not be able to recover said log and might not ever even start > the log recovery process. > There's a bit of a balance here between handling dirty log + obfuscation cases where a log recovery issue is the purpose of the metadump, it is not the purpose and the original fs can run log recovery without disrupting problem diagnosis, or something in between and the resulting image is explicitly verified to not lead to such metadump-specific corruption. I think adjusting the preexisting warning wrt to the fact that log data is not obfuscated to point out this issue is a relatively minor change given the potential result.. (put another way, it seems kind of silly that we'd warn about leaking unobfuscated data but not this..). Brian > --D > > > > > > I suppose we could also just exit under such conditions unless the user > > > passes a force flag or something. Maybe that's overkill too, though.. > > > > yeah, let's not overengineer - dumping a dirty, obfuscated log is quite > > typical I think. > > > > -Eric > > -- > > 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] metadump: warn about corruption if log is dirty 2017-04-12 11:03 ` Brian Foster @ 2017-04-12 11:24 ` Jan Tulak 0 siblings, 0 replies; 21+ messages in thread From: Jan Tulak @ 2017-04-12 11:24 UTC (permalink / raw) To: Brian Foster; +Cc: Darrick J. Wong, Eric Sandeen, linux-xfs On Wed, Apr 12, 2017 at 1:03 PM, Brian Foster <bfoster@redhat.com> wrote: > On Tue, Apr 11, 2017 at 04:44:44PM -0700, Darrick J. Wong wrote: >> On Tue, Apr 11, 2017 at 02:01:41PM -0500, Eric Sandeen wrote: >> > On 4/11/17 1:43 PM, Brian Foster wrote: >> > > On Tue, Apr 11, 2017 at 01:34:25PM -0500, Eric Sandeen wrote: >> > >> On 4/11/17 1:30 PM, Brian Foster wrote: >> > >>> On Tue, Apr 11, 2017 at 04:12:36PM +0200, Jan Tulak wrote: >> > >>>> Add a warning about possible corruption when exporting a dirty log, as >> > >>>> the log content does not agree with obfuscated metadata. >> > >>>> >> > >>>> Signed-off-by: Jan Tulak <jtulak@redhat.com> >> > >>>> --- >> > >>> >> > >>> Thanks for posting this... >> > >>> >> > >>>> db/metadump.c | 3 ++- >> > >>>> 1 file changed, 2 insertions(+), 1 deletion(-) >> > >>>> >> > >>>> diff --git a/db/metadump.c b/db/metadump.c >> > >>>> index 66952f6..74e24b2 100644 >> > >>>> --- a/db/metadump.c >> > >>>> +++ b/db/metadump.c >> > >>>> @@ -2726,7 +2726,8 @@ copy_log(void) >> > >>>> /* keep the dirty log */ >> > >>>> if (obfuscate) >> > >>>> print_warning( >> > >>>> -_("Filesystem log is dirty; image will contain unobfuscated metadata in log.")); >> > >>>> +_("Filesystem log is dirty; image will contain unobfuscated metadata in log " >> > >>>> + "and a log replay can cause a corruption.")); >> > >>> >> > >>> I think a slightly more verbose message might be a good idea. For >> > >>> example, something like the following: >> > >>> >> > >>> "Filesystem log is dirty; image will contain unobfuscated metadata in >> > >>> the log. Log recovery of an obfuscated image can cause filesystem >> > >>> corruption. Please mount the source image to clean the log or disable >> > >>> metadump obfuscation." >> > >>> >> > >>> That could also say "... or verify that log recovery of the resulting >> > >>> image does not cause corruption," but that might be overkill. Thoughts? >> > >>> Eric? >> > >> >> > >> I think we do need a good explanation, but that will take a lot of workd. >> > >> We could also refer to the man page for more details - it's getting pretty >> > >> long for a warning from the tool. >> > >> >> > > >> > > Hm, yeah. Maybe the existing warning can be condensed a bit more to >> > > something like: >> > > >> > > "Warning: log recovery of an obfuscated metadata image can leak >> > > unobfuscated metadata and/or cause filesystem corruption. Please mount >> > > the source image to clean the log or disable obfuscation." >> > >> > s/filesystem corruption/image corruption/ - we don't want anyone to think >> > that it damaged the original fs! >> >> OTOH the fs might be damaged just badly enough that log recovery is >> impossible (which is why we're creating the metadump to send to support) >> so aborting metadump is the wrong thing to do. >> > > Indeed.. > >> It seems sort of silly even to lecture the user about log recovery when >> they might not be able to recover said log and might not ever even start >> the log recovery process. >> > > There's a bit of a balance here between handling dirty log + obfuscation > cases where a log recovery issue is the purpose of the metadump, it is > not the purpose and the original fs can run log recovery without > disrupting problem diagnosis, or something in between and the resulting > image is explicitly verified to not lead to such metadump-specific > corruption. > > I think adjusting the preexisting warning wrt to the fact that log data > is not obfuscated to point out this issue is a relatively minor change > given the potential result.. (put another way, it seems kind of silly > that we'd warn about leaking unobfuscated data but not this..). > > Brian > I think that this is something we really should notify about. It serves as a notification for a rare issue, "hey, are you aware of this," rather than a list of instructions or a manual. So, I would keep the message short. This (slightly modified) variant sounds good to me: "Warning: log recovery of an obfuscated metadata image can leak unobfuscated metadata and/or cause image corruption. Please mount the source image to clean the log or disable obfuscation, if possible." Jan -- Jan Tulak jtulak@redhat.com / jan@tulak.me ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2017-06-15 11:23 UTC | newest] Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-04-13 8:13 [PATCH 0/2 v2] xfsprogs: metadump warns about dirty journal Jan Tulak 2017-04-13 8:13 ` [PATCH 1/2] metadump: warn about corruption if log is dirty Jan Tulak 2017-04-13 11:54 ` Brian Foster 2017-06-15 0:06 ` Eric Sandeen 2017-06-15 11:23 ` Brian Foster 2017-04-13 8:13 ` [PATCH 2/2] xfsprogs: update man for metadump about dirty log/obfuscation issue Jan Tulak 2017-04-13 12:01 ` Brian Foster 2017-04-13 12:29 ` Jan Tulak 2017-04-13 13:04 ` Brian Foster 2017-04-13 13:49 ` Eric Sandeen 2017-04-13 15:50 ` Darrick J. Wong 2017-04-13 17:01 ` Eric Sandeen 2017-04-13 17:06 ` Darrick J. Wong -- strict thread matches above, loose matches on Subject: below -- 2017-04-11 14:12 [PATCH 0/2] xfsprogs: metadump/mdrestore warns about dirty journal Jan Tulak 2017-04-11 14:12 ` [PATCH 1/2] metadump: warn about corruption if log is dirty Jan Tulak 2017-04-11 18:30 ` Brian Foster 2017-04-11 18:34 ` Eric Sandeen 2017-04-11 18:43 ` Brian Foster 2017-04-11 19:01 ` Eric Sandeen 2017-04-11 23:44 ` Darrick J. Wong 2017-04-12 11:03 ` Brian Foster 2017-04-12 11:24 ` Jan Tulak
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.