[PATCH 1/1] btrfs: fix NPD when target device is missing

[PATCH 1/1] btrfs: fix NPD when target device is missing

[Anand Jain]

The replace target device can be missing in which case we don't
allocate a missing btrfs_device when mounted with the -o degraded.
So check the device before access.

BUG: unable to handle kernel NULL pointer dereference at 00000000000000b0
IP: btrfs_destroy_dev_replace_tgtdev+0x43/0xf0 [btrfs]
Call Trace:
btrfs_dev_replace_cancel+0x15f/0x180 [btrfs]
btrfs_ioctl+0x2216/0x2590 [btrfs]
do_vfs_ioctl+0x625/0x650
SyS_ioctl+0x4e/0x80
do_syscall_64+0x5d/0x160
entry_SYSCALL64_slow_path+0x25/0x25

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/dev-replace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
index dbaa6880a15e..87f975143c05 100644
--- a/fs/btrfs/dev-replace.c
+++ b/fs/btrfs/dev-replace.c
@@ -312,7 +312,7 @@ void btrfs_after_dev_replace_commit(struct btrfs_fs_info *fs_info)
 
 static char* btrfs_dev_name(struct btrfs_device *device)
 {
-	if (test_bit(BTRFS_DEV_STATE_MISSING, &device->dev_state))
+	if (!device || test_bit(BTRFS_DEV_STATE_MISSING, &device->dev_state))
 		return "<missing disk>";
 	else
 		return rcu_str_deref(device->name);
-- 
2.15.0

Re: [PATCH 1/1] btrfs: fix NPD when target device is missing

[David Sterba]

On Tue, Feb 20, 2018 at 10:46:25PM +0800, Anand Jain wrote:
> The replace target device can be missing in which case we don't
> allocate a missing btrfs_device when mounted with the -o degraded.
> So check the device before access.
> 
> BUG: unable to handle kernel NULL pointer dereference at 00000000000000b0

Please don't use uncommon acronyms, NPD is quite confusing, null pointer
deref should be fine.

> IP: btrfs_destroy_dev_replace_tgtdev+0x43/0xf0 [btrfs]
> Call Trace:
> btrfs_dev_replace_cancel+0x15f/0x180 [btrfs]
> btrfs_ioctl+0x2216/0x2590 [btrfs]
> do_vfs_ioctl+0x625/0x650
> SyS_ioctl+0x4e/0x80
> do_syscall_64+0x5d/0x160
> entry_SYSCALL64_slow_path+0x25/0x25

Do you have a reproducer for that?

[PATCH v2] btrfs: fix null pointer deref when target device is missing

[Anand Jain]

The replace target device can be missing when mounted with -o degraded,
but we wont allocate a missing btrfs_device to it. So check the device
before access.

BUG: unable to handle kernel NULL pointer dereference at 00000000000000b0
IP: btrfs_destroy_dev_replace_tgtdev+0x43/0xf0 [btrfs]
Call Trace:
btrfs_dev_replace_cancel+0x15f/0x180 [btrfs]
btrfs_ioctl+0x2216/0x2590 [btrfs]
do_vfs_ioctl+0x625/0x650
SyS_ioctl+0x4e/0x80
do_syscall_64+0x5d/0x160
entry_SYSCALL64_slow_path+0x25/0x25

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
v1->v2: Fix change log. Fix $subject.
  Old $subject
    btrfs: fix NPD when target device is missing

 fs/btrfs/dev-replace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
index c97969b2abbd..e279f04b3388 100644
--- a/fs/btrfs/dev-replace.c
+++ b/fs/btrfs/dev-replace.c
@@ -312,7 +312,7 @@ void btrfs_after_dev_replace_commit(struct btrfs_fs_info *fs_info)
 
 static char* btrfs_dev_name(struct btrfs_device *device)
 {
-	if (test_bit(BTRFS_DEV_STATE_MISSING, &device->dev_state))
+	if (!device || test_bit(BTRFS_DEV_STATE_MISSING, &device->dev_state))
 		return "<missing disk>";
 	else
 		return rcu_str_deref(device->name);
-- 
2.15.0

Re: [PATCH 1/1] btrfs: fix NPD when target device is missing

[Anand Jain]

On 02/24/2018 06:55 AM, David Sterba wrote:
> On Tue, Feb 20, 2018 at 10:46:25PM +0800, Anand Jain wrote:
>> The replace target device can be missing in which case we don't
>> allocate a missing btrfs_device when mounted with the -o degraded.
>> So check the device before access.
>>
>> BUG: unable to handle kernel NULL pointer dereference at 00000000000000b0
> 
> Please don't use uncommon acronyms, NPD is quite confusing, null pointer
> deref should be fine.

  Ok, sent v2.

>> IP: btrfs_destroy_dev_replace_tgtdev+0x43/0xf0 [btrfs]
>> Call Trace:
>> btrfs_dev_replace_cancel+0x15f/0x180 [btrfs]
>> btrfs_ioctl+0x2216/0x2590 [btrfs]
>> do_vfs_ioctl+0x625/0x650
>> SyS_ioctl+0x4e/0x80
>> do_syscall_64+0x5d/0x160
>> entry_SYSCALL64_slow_path+0x25/0x25
> 
> Do you have a reproducer for that?

  For now, I used a tweaked btrfs.ko [1], then

  mkfs.btrfs -fq /dev/sdb && mount /dev/sdb /btrfs
  btrfs rep start -B /dev/sdb /dev/sdc
    after reboot, we have the replace target device
    and now use non-tweaked btrfs.ko
  mount -o degraded /dev/sdb /btrfs

[1]
-----------
diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
index 476981c2cf55..8ea4856b6368 100644
--- a/fs/btrfs/dev-replace.c
+++ b/fs/btrfs/dev-replace.c
@@ -25,6 +25,7 @@
  #include <linux/capability.h>
  #include <linux/kthread.h>
  #include <linux/math64.h>
+#include <linux/reboot.h>
  #include <asm/div64.h>
  #include "ctree.h"
  #include "extent_map.h"
@@ -419,6 +420,8 @@ int btrfs_dev_replace_start(struct btrfs_fs_info 
*fs_info,
                               btrfs_device_get_total_bytes(src_device),
                               &dev_replace->scrub_progress, 0, 1);

+       emergency_restart();
+
         ret = btrfs_dev_replace_finishing(fs_info, ret);
         if (ret == -EINPROGRESS) {
                 ret = BTRFS_IOCTL_DEV_REPLACE_RESULT_SCRUB_INPROGRESS;
------------


Thanks, Anand

> --
> 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
> 

Re: [PATCH 1/1] btrfs: fix NPD when target device is missing

[David Sterba]

On Sat, Feb 24, 2018 at 10:07:33PM +0800, Anand Jain wrote:
> >> IP: btrfs_destroy_dev_replace_tgtdev+0x43/0xf0 [btrfs]
> >> Call Trace:
> >> btrfs_dev_replace_cancel+0x15f/0x180 [btrfs]
> >> btrfs_ioctl+0x2216/0x2590 [btrfs]
> >> do_vfs_ioctl+0x625/0x650
> >> SyS_ioctl+0x4e/0x80
> >> do_syscall_64+0x5d/0x160
> >> entry_SYSCALL64_slow_path+0x25/0x25
> > 
> > Do you have a reproducer for that?
> 
>   For now, I used a tweaked btrfs.ko [1], then
> 
>   mkfs.btrfs -fq /dev/sdb && mount /dev/sdb /btrfs
>   btrfs rep start -B /dev/sdb /dev/sdc
>     after reboot, we have the replace target device
>     and now use non-tweaked btrfs.ko
>   mount -o degraded /dev/sdb /btrfs
> 
> [1]
> -----------
> diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
> index 476981c2cf55..8ea4856b6368 100644
> --- a/fs/btrfs/dev-replace.c
> +++ b/fs/btrfs/dev-replace.c
> @@ -25,6 +25,7 @@
>   #include <linux/capability.h>
>   #include <linux/kthread.h>
>   #include <linux/math64.h>
> +#include <linux/reboot.h>
>   #include <asm/div64.h>
>   #include "ctree.h"
>   #include "extent_map.h"
> @@ -419,6 +420,8 @@ int btrfs_dev_replace_start(struct btrfs_fs_info 
> *fs_info,
>                                btrfs_device_get_total_bytes(src_device),
>                                &dev_replace->scrub_progress, 0, 1);
> 
> +       emergency_restart();

Ok, not something that we can easily turn into a regression test.

I'll reorder this fix before patch "btrfs: log, when replace, is
canceled by the user", so it is bisectable. Thanks.