| Message ID | 1412044762-2480-1-git-send-email-quwenruo@cn.fujitsu.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
On Tue, Sep 30, 2014 at 10:39:22AM +0800, Qu Wenruo wrote: > --- a/disk-io.c > +++ b/disk-io.c > @@ -1134,7 +1134,8 @@ static struct btrfs_fs_info *__open_ctree_fd(int fp, const char *path, > return fs_info; > > out_failed: > - if (flags & OPEN_CTREE_PARTIAL) > + if (flags & OPEN_CTREE_PARTIAL && > + fs_info->tree_root && fs_info->fs_root) > return fs_info; I see a conflict with a pending patch https://patchwork.kernel.org/patch/4254631/ that removes the check completely but fixes the crash in another way. I like Wang's patch because it keeps the logic about partial open inside btrfs_setup_all_roots(). Please test if it fixes the crash with the corrupted image you have. Thanks. > out_chunk: > btrfs_release_all_roots(fs_info); -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> btrfs_setup_all_roots(). Please test if it fixes the crash with the > corrupted image you have. Thanks. Perhaps with a test in xfstests. - z -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Sorry for the late reply. Wang's patch fixed all the NULL tree root related bugs. So my patches are not needed and please ignore them. I'll also reply to my patches to mark them unneeded. Thanks, Qu -------- Original Message -------- Subject: Re: [PATCH] btrfs-progs: Don't return any fs_info that contain NULL tree_root or fs_root. From: David Sterba <dsterba@suse.cz> To: Qu Wenruo <quwenruo@cn.fujitsu.com> Date: 2014?09?30? 19:35 > On Tue, Sep 30, 2014 at 10:39:22AM +0800, Qu Wenruo wrote: >> --- a/disk-io.c >> +++ b/disk-io.c >> @@ -1134,7 +1134,8 @@ static struct btrfs_fs_info *__open_ctree_fd(int fp, const char *path, >> return fs_info; >> >> out_failed: >> - if (flags & OPEN_CTREE_PARTIAL) >> + if (flags & OPEN_CTREE_PARTIAL && >> + fs_info->tree_root && fs_info->fs_root) >> return fs_info; > I see a conflict with a pending patch > https://patchwork.kernel.org/patch/4254631/ > > that removes the check completely but fixes the crash in another way. I > like Wang's patch because it keeps the logic about partial open inside > btrfs_setup_all_roots(). Please test if it fixes the crash with the > corrupted image you have. Thanks. > >> out_chunk: >> btrfs_release_all_roots(fs_info); > -- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" 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-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Please ignore this patch since Wang's patch has already fixed them. https://patchwork.kernel.org/patch/4254631/ Thanks, Qu -------- Original Message -------- Subject: [PATCH] btrfs-progs: Don't return any fs_info that contain NULL tree_root or fs_root. From: Qu Wenruo <quwenruo@cn.fujitsu.com> To: <linux-btrfs@vger.kernel.org> Date: 2014?09?30? 10:39 > [BUG] > btrfsck will segfault if it fails to open the fs tree or tree root. > > [REPRODUCER] > Execute btrfsck on a highly damaged btrfs image. > fsfuzz can be used to make a junk btrfs image. > > [REASON] > Current open_ctree() in btrfs-progs support OPEN_CTREE_PARTIAL flag to > allow return fs_info even some of the trees is missing. > > However it is too loose and even allows fs_info containing no tree to be > returned. > > And when it happens, fs_info->fs_root is NULL, > close_ctree(fs_info->fs_root) will cause the access to NULL pointer and > segfault. > > [FIX] > This patch will add checks for fs_info->tree_root and fs_info->fs_root > before return fs_info. > > Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com> > --- > disk-io.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/disk-io.c b/disk-io.c > index 26a532e..21a3083 100644 > --- a/disk-io.c > +++ b/disk-io.c > @@ -1134,7 +1134,8 @@ static struct btrfs_fs_info *__open_ctree_fd(int fp, const char *path, > return fs_info; > > out_failed: > - if (flags & OPEN_CTREE_PARTIAL) > + if (flags & OPEN_CTREE_PARTIAL && > + fs_info->tree_root && fs_info->fs_root) > return fs_info; > out_chunk: > btrfs_release_all_roots(fs_info); -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/disk-io.c b/disk-io.c index 26a532e..21a3083 100644 --- a/disk-io.c +++ b/disk-io.c @@ -1134,7 +1134,8 @@ static struct btrfs_fs_info *__open_ctree_fd(int fp, const char *path, return fs_info; out_failed: - if (flags & OPEN_CTREE_PARTIAL) + if (flags & OPEN_CTREE_PARTIAL && + fs_info->tree_root && fs_info->fs_root) return fs_info; out_chunk: btrfs_release_all_roots(fs_info);
[BUG] btrfsck will segfault if it fails to open the fs tree or tree root. [REPRODUCER] Execute btrfsck on a highly damaged btrfs image. fsfuzz can be used to make a junk btrfs image. [REASON] Current open_ctree() in btrfs-progs support OPEN_CTREE_PARTIAL flag to allow return fs_info even some of the trees is missing. However it is too loose and even allows fs_info containing no tree to be returned. And when it happens, fs_info->fs_root is NULL, close_ctree(fs_info->fs_root) will cause the access to NULL pointer and segfault. [FIX] This patch will add checks for fs_info->tree_root and fs_info->fs_root before return fs_info. Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com> --- disk-io.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)