[PATCH] btrfs: Handle error of get_old_root

[PATCH] btrfs: Handle error of get_old_root

[Nikolay Borisov]

In btrfs_search_old_slot get_old_root is always used with the
assumption it cannot fail. However, this is not true in rare
circumstance it can fail and return null. This will lead to null
point dereference when the header is read. Fix this by checking the
return value and properly handling NULL by setting ret to -EIO and
returning gracefully.

CID: 1087503
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 fs/btrfs/ctree.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index 1124d236291d..a5399fd49c17 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -2961,6 +2961,10 @@ int btrfs_search_old_slot(struct btrfs_root *root, const struct btrfs_key *key,
 
 again:
 	b = get_old_root(root, time_seq);
+	if (!b) {
+		ret = -EIO;
+		goto done;
+	}
 	level = btrfs_header_level(b);
 	p->locks[level] = BTRFS_READ_LOCK;
 
-- 
2.7.4

Re: [PATCH] btrfs: Handle error of get_old_root

[Lu Fengqi]

On Thu, Sep 13, 2018 at 11:35:10AM +0300, Nikolay Borisov wrote:
>In btrfs_search_old_slot get_old_root is always used with the
>assumption it cannot fail. However, this is not true in rare
>circumstance it can fail and return null. This will lead to null
>point dereference when the header is read. Fix this by checking the
>return value and properly handling NULL by setting ret to -EIO and
>returning gracefully.
>
>CID: 1087503
>Signed-off-by: Nikolay Borisov <nborisov@suse.com>

Reviewed-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>

-- 
Thanks,
Lu

>---
> fs/btrfs/ctree.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
>diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
>index 1124d236291d..a5399fd49c17 100644
>--- a/fs/btrfs/ctree.c
>+++ b/fs/btrfs/ctree.c
>@@ -2961,6 +2961,10 @@ int btrfs_search_old_slot(struct btrfs_root *root, const struct btrfs_key *key,
> 
> again:
> 	b = get_old_root(root, time_seq);
>+	if (!b) {
>+		ret = -EIO;
>+		goto done;
>+	}
> 	level = btrfs_header_level(b);
> 	p->locks[level] = BTRFS_READ_LOCK;
> 
>-- 
>2.7.4
>
>
>

Re: [PATCH] btrfs: Handle error of get_old_root

[David Sterba]

On Thu, Sep 13, 2018 at 11:35:10AM +0300, Nikolay Borisov wrote:
> In btrfs_search_old_slot get_old_root is always used with the
> assumption it cannot fail. However, this is not true in rare
> circumstance it can fail and return null.

Currently this will not happen, as the code returning NULL

1383         if (!eb)
1384                 return NULL;

is preceed by btrfs_clone_extent_buffer or alloc_dummy_extent_buffer
that will not fail due to GFP_NOFAIL in __alloc_extent_buffer.

However I agree the error handling in btrfs_search_old_slot should be
there as the NOFAIL semantics is hidden and may change eventually.