linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] btrfs: ioctl: fix inaccurate determination of exclusive_operation
@ 2023-03-24  3:16 xiaoshoukui
  2023-03-27 23:05 ` David Sterba
  0 siblings, 1 reply; 7+ messages in thread
From: xiaoshoukui @ 2023-03-24  3:16 UTC (permalink / raw)
  To: clm, josef, dsterba; +Cc: linux-btrfs, linux-kernel, xiaoshoukui, xiaoshoukui

with fs_info->exclusive_operation == BTRFS_EXCLOP_DEV_ADD enter
btrfs_ioctl_add_dev function , exclusive_operation will be classified
as in paused balance operation. After return from btrfs_ioctl_add_dev,
exclusive_operation will be restore to BTRFS_EXCLOP_BALANCE_PAUSED which
is not its original state.

Signed-off-by: xiaoshoukui <xiaoshoukui@ruijie.com.cn>
---
 fs/btrfs/ioctl.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index a0ef1a1784c7..aab5fdb9445c 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -2629,7 +2629,7 @@ static long btrfs_ioctl_add_dev(struct btrfs_fs_info *fs_info, void __user *arg)
 	}
 
 	if (!btrfs_exclop_start(fs_info, BTRFS_EXCLOP_DEV_ADD)) {
-		if (!btrfs_exclop_start_try_lock(fs_info, BTRFS_EXCLOP_DEV_ADD))
+		if (fs_info->exclusive_operation != BTRFS_EXCLOP_BALANCE_PAUSED)
 			return BTRFS_ERROR_DEV_EXCL_RUN_IN_PROGRESS;
 
 		/*
@@ -2637,8 +2637,9 @@ static long btrfs_ioctl_add_dev(struct btrfs_fs_info *fs_info, void __user *arg)
 		 * change the exclusive op type and remember we should bring
 		 * back the paused balance
 		 */
+		spin_lock(&fs_info->super_lock);
 		fs_info->exclusive_operation = BTRFS_EXCLOP_DEV_ADD;
-		btrfs_exclop_start_unlock(fs_info);
+		spin_unlock(&fs_info->super_lock);
 		restore_op = true;
 	}
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] btrfs: ioctl: fix inaccurate determination of exclusive_operation
  2023-03-24  3:16 [PATCH] btrfs: ioctl: fix inaccurate determination of exclusive_operation xiaoshoukui
@ 2023-03-27 23:05 ` David Sterba
  0 siblings, 0 replies; 7+ messages in thread
From: David Sterba @ 2023-03-27 23:05 UTC (permalink / raw)
  To: xiaoshoukui; +Cc: clm, josef, dsterba, linux-btrfs, linux-kernel, xiaoshoukui

On Thu, Mar 23, 2023 at 11:16:11PM -0400, xiaoshoukui wrote:
> with fs_info->exclusive_operation == BTRFS_EXCLOP_DEV_ADD enter
> btrfs_ioctl_add_dev function , exclusive_operation will be classified
> as in paused balance operation. After return from btrfs_ioctl_add_dev,
> exclusive_operation will be restore to BTRFS_EXCLOP_BALANCE_PAUSED which
> is not its original state.

Sorry, I don't understand what you mean. The paused balance and 'device
add' are supposed to be compatible exclusive operations (see commit
a174c0a2e857 ("btrfs: allow device add if balance is paused")).

Have you found some bug with the above or is there other combination of
the exclusive operations that should not work? The changes to the state
values are the same, besides the wrong locking.

> Signed-off-by: xiaoshoukui <xiaoshoukui@ruijie.com.cn>
> ---
>  fs/btrfs/ioctl.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index a0ef1a1784c7..aab5fdb9445c 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -2629,7 +2629,7 @@ static long btrfs_ioctl_add_dev(struct btrfs_fs_info *fs_info, void __user *arg)
>  	}
>  
>  	if (!btrfs_exclop_start(fs_info, BTRFS_EXCLOP_DEV_ADD)) {
> -		if (!btrfs_exclop_start_try_lock(fs_info, BTRFS_EXCLOP_DEV_ADD))
> +		if (fs_info->exclusive_operation != BTRFS_EXCLOP_BALANCE_PAUSED)

This is removing the atomicity of the check so it's racy and could
forcibly overwrite the exclusive operation to BTRFS_EXCLOP_DEV_ADD
without the protecting the whole critical section.

>  			return BTRFS_ERROR_DEV_EXCL_RUN_IN_PROGRESS;
>  
>  		/*
> @@ -2637,8 +2637,9 @@ static long btrfs_ioctl_add_dev(struct btrfs_fs_info *fs_info, void __user *arg)
>  		 * change the exclusive op type and remember we should bring
>  		 * back the paused balance
>  		 */
> +		spin_lock(&fs_info->super_lock);

What if there's another exclusive operation started before this lock is
taken?

>  		fs_info->exclusive_operation = BTRFS_EXCLOP_DEV_ADD;
> -		btrfs_exclop_start_unlock(fs_info);
> +		spin_unlock(&fs_info->super_lock);
>  		restore_op = true;
>  	}

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] btrfs: ioctl: fix inaccurate determination of exclusive_operation
  2023-04-04 19:10   ` David Sterba
@ 2023-04-06  6:58     ` xiaoshoukui
  0 siblings, 0 replies; 7+ messages in thread
From: xiaoshoukui @ 2023-04-06  6:58 UTC (permalink / raw)
  To: dsterba; +Cc: clm, josef, dsterba, linux-btrfs, linux-kernel, xiaoshoukui

> Please send a fix with the analysis you dit and add the relevant parts
> of stack traces. The reproducer would be good to have in fstests, for
> the changelog please describe the conditions that could trigger the
> assertion, the reproducer itself is too long. Thanks.

Already posted the fix patch in a new thread. Thanks for your advice on
reproducer program. It will take some time to refactor it to fstests testcase.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] btrfs: ioctl: fix inaccurate determination of exclusive_operation
  2023-04-03  9:37 ` xiaoshoukui
@ 2023-04-04 19:10   ` David Sterba
  2023-04-06  6:58     ` xiaoshoukui
  0 siblings, 1 reply; 7+ messages in thread
From: David Sterba @ 2023-04-04 19:10 UTC (permalink / raw)
  To: xiaoshoukui; +Cc: clm, josef, dsterba, linux-btrfs, linux-kernel, xiaoshoukui

On Mon, Apr 03, 2023 at 05:37:57AM -0400, xiaoshoukui wrote:
> > Yeah I think the assertion should also check for NONE status. The paused
> > balance makes the state tracking harder but in user-started (manual or
> > scripted) commands it's typically not racing.
> 
> An assertion failure means that the code may not have taken careful consideration.
> After I patched the BTRFS_EXCLOP_NONE to the assertion, regression tests shows that
> another scenario I missed.
> 
> With started state == BTRFS_EXCLOP_BALANCE_PAUSED, cocurrently adding multiple devices
> to the same mount point and btrfs_exclop_balance executed finish before the latter
> thread execute assertion in btrfs_exclop_balance, exclusive_operation will changed to 
> BTRFS_EXCLOP_BALANCE_PAUSED state. 
> 
> I also added btrfs_info before ASSERT to help troubleshooting:
> > btrfs_info(fs_info, "fs_info exclusive_operation: %d",
> >        fs_info->exclusive_operation);
> > ASSERT(fs_info->exclusive_operation == BTRFS_EXCLOP_BALANCE ||
> >        fs_info->exclusive_operation == BTRFS_EXCLOP_DEV_ADD ||
> >        fs_info->exclusive_operation == BTRFS_EXCLOP_NONE);

> I think the assertion should also check for BTRFS_EXCLOP_BALANCE_PAUSED status.

Agreed and this looks like the complete set of the compatible
operations. I hope this does not enable some combination that should not
be allowed but the enabling side does the try lock and allows only the
paused + dev_add combination.

Please send a fix with the analysis you dit and add the relevant parts
of stack traces. The reproducer would be good to have in fstests, for
the changelog please describe the conditions that could trigger the
assertion, the reproducer itself is too long. Thanks.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] btrfs: ioctl: fix inaccurate determination of exclusive_operation
       [not found] <20230331174533.GZ10580 () twin ! jikos ! cz>
@ 2023-04-03  9:37 ` xiaoshoukui
  2023-04-04 19:10   ` David Sterba
  0 siblings, 1 reply; 7+ messages in thread
From: xiaoshoukui @ 2023-04-03  9:37 UTC (permalink / raw)
  To: dsterba; +Cc: clm, josef, dsterba, linux-btrfs, linux-kernel, xiaoshoukui

> Yeah I think the assertion should also check for NONE status. The paused
> balance makes the state tracking harder but in user-started (manual or
> scripted) commands it's typically not racing.

An assertion failure means that the code may not have taken careful consideration.
After I patched the BTRFS_EXCLOP_NONE to the assertion, regression tests shows that
another scenario I missed.

With started state == BTRFS_EXCLOP_BALANCE_PAUSED, cocurrently adding multiple devices
to the same mount point and btrfs_exclop_balance executed finish before the latter
thread execute assertion in btrfs_exclop_balance, exclusive_operation will changed to 
BTRFS_EXCLOP_BALANCE_PAUSED state. 

I also added btrfs_info before ASSERT to help troubleshooting:
> btrfs_info(fs_info, "fs_info exclusive_operation: %d",
>        fs_info->exclusive_operation);
> ASSERT(fs_info->exclusive_operation == BTRFS_EXCLOP_BALANCE ||
>        fs_info->exclusive_operation == BTRFS_EXCLOP_DEV_ADD ||
>        fs_info->exclusive_operation == BTRFS_EXCLOP_NONE);

Regression test log as follow:
The enum value of 1 correspond to BTRFS_EXCLOP_BALANCE_PAUSED:
root@syzkaller:/home/xsk# ./repro 
Failed to add device /dev/vda, errno 14
Failed to add device /dev/vda, errno 14
Failed to add device /dev/vda, errno 14
Failed to add device /dev/vda, errno 14
Failed to add device /dev/vda, errno 14
Failed to add device /dev/vda, errno 14
Failed to add device /dev/vda, errno 14
Failed to add device /dev/vda, errno 14
Failed to add de[  416.611428][ T7970] BTRFS info (device loop0): fs_info exclusive_operation: 0
vice /dev/vda, errno 14
Failed to add device /dev/vda, errno 14
[  416.613973][ T7971] BTRFS info (device loop0): fs_info exclusive_operation: 3
Failed to add device /dev/vda, errno 14
[  416.615456][ T7972] BTRFS info (device loop0): fs_info exclusive_operation: 3
Failed to add device /dev/vda, errno 14
[  416.617528][ T7973] BTRFS info (device loop0): fs_info exclusive_operation: 3
Failed to add de[  416.618359][ T7974] BTRFS info (device loop0): fs_info exclusive_operation: 3
vice /dev/vda, errno 14
Failed to add device /dev/vda, errno 14
[  416.622589][ T7975] BTRFS info (device loop0): fs_info exclusive_operation: 3
Failed to add device /dev/vda, e[  416.624034][ T7976] BTRFS info (device loop0): fs_info exclusive_operation: 3
rrno 14
Failed to add device /dev/vda, errno 14
[  416.626420][ T7977] BTRFS info (device loop0): fs_info exclusive_operation: 3
Failed to add device /dev/vda, errno 14
[  416.627643][ T7978] BTRFS info (device loop0): fs_info exclusive_operation: 3
Failed to add device /dev/vda, errno 14
[  416.629006][ T7979] BTRFS info (device loop0): fs_info exclusive_operation: 3
[  416.630298][ T7980] BTRFS info (device loop0): fs_info exclusive_operation: 3
Failed to add device /dev/vda, errno 14
Failed to add device /dev/vda, errno 14
[  416.632787][ T7981] BTRFS info (device loop0): fs_info exclusive_operation: 3
Failed to add device /dev/vda, errno 14
[  416.634282][ T7982] BTRFS info (device loop0): fs_info exclusive_operation: 3
Failed to add device /dev/vda, errno 14
[  416.636202][ T7983] BTRFS info (device loop0): fs_info exclusive_operation: 3
[  416.637012][ T7984] BTRFS info (device loop0): fs_info exclusive_operation: 1
Failed to add de[  416.637759][ T7984] assertion failed: fs_info->exclusive_operation == BTRFS_EXCLOP_BALANCE || fs_info->exclusive_operation == BTRFS_EXCLOP_DEV_ADD || fs_info->exclusive_operation == BTRFS_EXCLOP_NONE, in fs/btrfs/ioctl.c:458
vice /dev/vda, [e  416.639845][ T7984] invalid opcode: 0000 [#1] PREEMPT SMP KASAN
rrno [1 4 416.640485][ T7984] CPU: 0 PID: 7984 Comm: repro Not tainted 6.2.0 #7
[  416.641172][ T7984] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014
[  416.642090][ T7984] RIP: 0010:btrfs_assertfail+0x2c/0x2e
[  416.642629][ T7984] Code: 1f 00 41 55 41 89 d5 41 54 49 89 f4 55 48 89 fd e8 b9 84 e8 f7 44 89 e9 4c 89 e2 48 89 ee 48 c7 c7 c0 af 54 8a e8 cb 49 f3 ff <0f> 0b 66 0f 1f 00 55 48 89 fd e8 95 84 e8 f7 48 89 ef 5d 48 c7 c6

[  416.644423][ T7984] RSP: 0018:ffffc90003ea7e28 EFLAGS: 00010282
[  416.645018][ T7984] RAX: 00000000000000cc RBX: 0000000000000000 RCX: 0000000000000000
[  416.645763][ T7984] RDX: ffff88801d030000 RSI: ffffffff81637e7c RDI: fffff520007d4fb7
[  416.646554][ T7984] RBP: ffffffff8a533de0 R08: 00000000000000cc R09: 0000000000000000
[  416.647299][ T7984] R10: 0000000000000001 R11: 0000000000000001 R12: ffffffff8a533da0
[  416.648041][ T7984] R13: 00000000000001ca R14: 000000005000940a R15: 0000000000000000
[  416.648785][ T7984] FS:  00007fa2985d4640(0000) GS:ffff88802cc00000(0000) knlGS:0000000000000000
[  416.649616][ T7984] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  416.650238][ T7984] CR2: 0000000000000000 CR3: 0000000018e5e000 CR4: 0000000000750ef0
[  416.650980][ T7984] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[  416.651725][ T7984] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[  416.652502][ T7984] PKRU: 55555554
[  416.652888][ T7984] Call Trace:
[  416.653241][ T7984]  <TASK>
[  416.653527][ T7984]  btrfs_exclop_balance+0x240/0x410
[  416.654036][ T7984]  ? memdup_user+0xab/0xc0
[  416.654465][ T7984]  ? PTR_ERR+0x17/0x20
[  416.654874][ T7984]  btrfs_ioctl_add_dev+0x2ee/0x320
[  416.655380][ T7984]  btrfs_ioctl+0x9d5/0x10d0
[  416.655822][ T7984]  ? btrfs_ioctl_encoded_write+0xb80/0xb80
[  416.656400][ T7984]  __x64_sys_ioctl+0x197/0x210
[  416.656874][ T7984]  do_syscall_64+0x3c/0xb0
[  416.657346][ T7984]  entry_SYSCALL_64_after_hwframe+0x63/0xcd
[  416.657922][ T7984] RIP: 0033:0x4546af
[  416.658304][ T7984] Code: 00 48 89 44 24 18 31 c0 48 8d 44 24 60 c7 04 24 10 00 00 00 48 89 44 24 08 48 8d 44 24 20 48 89 44 24 10 b8 10 00 00 00 0f 05 <41> 89 c0 3d 00 f0 ff ff 77 1f 48 8b 44 24 18 64 48 2b 04 25 28 00
[  416.660170][ T7984] RSP: 002b:00007fa2985d4150 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
[  416.660972][ T7984] RAX: ffffffffffffffda RBX: 00007fa2985d4640 RCX: 00000000004546af
[  416.661714][ T7984] RDX: 0000000000000000 RSI: 000000005000940a RDI: 0000000000000003
[  416.662449][ T7984] RBP: 00007fa2985d41d0 R08: 0000000000000000 R09: 00007ffee37a4c4f
[  416.663195][ T7984] R10: 0000000000000000 R11: 0000000000000246 R12: 00007fa2985d4640
[  416.663951][ T7984] R13: 0000000000000009 R14: 000000000041b320 R15: 00007fa297dd4000
[  416.664703][ T7984]  </TASK>
[  416.665040][ T7984] Modules linked in:
[  416.665590][ T7984] ---[ end trace 0000000000000000 ]---
[  416.666176][ T7984] RIP: 0010:btrfs_assertfail+0x2c/0x2e
[  416.666774][ T7984] Code: 1f 00 41 55 41 89 d5 41 54 49 89 f4 55 48 89 fd e8 b9 84 e8 f7 44 89 e9 4c 89 e2 48 89 ee 48 c7 c7 c0 af 54 8a e8 cb 49 f3 ff <0f> 0b 66 0f 1f 00 55 48 89 fd e8 95 84 e8 f7 48 89 ef 5d 48 c7 c6
[  416.668775][ T7984] RSP: 0018:ffffc90003ea7e28 EFLAGS: 00010282
[  416.669425][ T7984] RAX: 00000000000000cc RBX: 0000000000000000 RCX: 0000000000000000
[  416.670235][ T7984] RDX: ffff88801d030000 RSI: ffffffff81637e7c RDI: fffff520007d4fb7
[  416.671050][ T7984] RBP: ffffffff8a533de0 R08: 00000000000000cc R09: 0000000000000000
[  416.671867][ T7984] R10: 0000000000000001 R11: 0000000000000001 R12: ffffffff8a533da0
[  416.672685][ T7984] R13: 00000000000001ca R14: 000000005000940a R15: 0000000000000000
[  416.673501][ T7984] FS:  00007fa2985d4640(0000) GS:ffff88802cc00000(0000) knlGS:0000000000000000
[  416.674425][ T7984] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  416.675114][ T7984] CR2: 0000000000000000 CR3: 0000000018e5e000 CR4: 0000000000750ef0
[  416.675933][ T7984] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[  416.676760][ T7984] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[  416.677544][ T7984] PKRU: 55555554
[  416.677885][ T7984] Kernel panic - not syncing: Fatal exception
[  416.678592][ T7984] Kernel Offset: disabled
[  416.679008][ T7984] Rebooting in 86400 seconds..


I think the assertion should also check for BTRFS_EXCLOP_BALANCE_PAUSED status.


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] btrfs: ioctl: fix inaccurate determination of exclusive_operation
  2023-03-28  9:43 ` xiaoshoukui
@ 2023-03-31 17:45   ` David Sterba
  0 siblings, 0 replies; 7+ messages in thread
From: David Sterba @ 2023-03-31 17:45 UTC (permalink / raw)
  To: xiaoshoukui
  Cc: dsterba, clm, josef, dsterba, linux-btrfs, linux-kernel, xiaoshoukui

On Tue, Mar 28, 2023 at 05:43:35AM -0400, xiaoshoukui wrote:
> > Have you found some bug with the above or is there other combination of
> > the exclusive operations that should not work? The changes to the state
> > values are the same, besides the wrong locking.
> 
> Yes, there is a racy bewteen btrfs_exclop_balance and btrfs_exclop_finish
> in btrfs_ioctl_add_dev, when cocurrently adding multiple devices to the
> same mnt point. That will cause the assertion in btrfs_exclop_balance to fail.
> 
> > void btrfs_exclop_balance(struct btrfs_fs_info *fs_info,
> > 			  enum btrfs_exclusive_operation op)
> > {
> > 	switch (op) {
> > 	case BTRFS_EXCLOP_BALANCE_PAUSED:
> > 		spin_lock(&fs_info->super_lock);
> > 		ASSERT(fs_info->exclusive_operation == BTRFS_EXCLOP_BALANCE ||
> > 		       fs_info->exclusive_operation == BTRFS_EXCLOP_DEV_ADD);
> 
> when btrfs_exclop_finish function was executed before the ASSERT, the
> fs_info->exclusive_operation will change to BTRFS_EXCLOP_NONE. So this
> assert will failed.
>
> Please review whether we should patch the assert to add BTRFS_EXCLOP_NONE condtion.
> I'll post a patch if needed. thx.

Yeah I think the assertion should also check for NONE status. The paused
balance makes the state tracking harder but in user-started (manual or
scripted) commands it's typically not racing.

btrfs_exclop_start_try_lock does not allow to do the change from
none -> op mandating an explicit btrfs_exclop_start first but the
assertions do not care about that.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] btrfs: ioctl: fix inaccurate determination of exclusive_operation
       [not found] <20230327230553.GJ10580 () twin ! jikos ! cz>
@ 2023-03-28  9:43 ` xiaoshoukui
  2023-03-31 17:45   ` David Sterba
  0 siblings, 1 reply; 7+ messages in thread
From: xiaoshoukui @ 2023-03-28  9:43 UTC (permalink / raw)
  To: dsterba; +Cc: clm, josef, dsterba, linux-btrfs, linux-kernel, xiaoshoukui

> Have you found some bug with the above or is there other combination of
> the exclusive operations that should not work? The changes to the state
> values are the same, besides the wrong locking.

Yes, there is a racy bewteen btrfs_exclop_balance and btrfs_exclop_finish
in btrfs_ioctl_add_dev, when cocurrently adding multiple devices to the
same mnt point. That will cause the assertion in btrfs_exclop_balance to fail.

> void btrfs_exclop_balance(struct btrfs_fs_info *fs_info,
> 			  enum btrfs_exclusive_operation op)
> {
> 	switch (op) {
> 	case BTRFS_EXCLOP_BALANCE_PAUSED:
> 		spin_lock(&fs_info->super_lock);
> 		ASSERT(fs_info->exclusive_operation == BTRFS_EXCLOP_BALANCE ||
> 		       fs_info->exclusive_operation == BTRFS_EXCLOP_DEV_ADD);

when btrfs_exclop_finish function was executed before the ASSERT, the
fs_info->exclusive_operation will change to BTRFS_EXCLOP_NONE. So this
assert will failed.


We can easily reproduce it with the following code.

#include <stdio.h>
#include <stdlib.h>
#include <fcntl.h>
#include <unistd.h>
#include <sys/ioctl.h>
#include <linux/btrfs.h>
#include <pthread.h>
#include <errno.h>
#include <dirent.h>
#include <string.h>

static int fd;
static int i;
static int controller = 0;
static int random_delay1 = 0;
static int random_delay2 = 0;
//const static int delta = 10000;


static pthread_t thread_id[10];
static char *path = "/dev/vda";
static char *btrfsmntpt = "/mnt/my_btrfs";


void prepare_random()
{
    random_delay1 = rand() % 80;
    random_delay2 = rand() % 20;
}

void *thr1(void *arg) {
    int ret;
    while(!controller);
    //usleep(delta);
    usleep(random_delay1);

    ret = ioctl(fd, BTRFS_IOC_ADD_DEV, 0);
    if (ret != 0) {
        printf("Failed to add device %s, errno %d\n", path, errno);
        return NULL;
    }
    printf("Device %s added successfully\n", path);
    return NULL;

}

void *thr2(void *arg) {
    int ret;
    while(!controller);
    usleep(random_delay2);
    ret = ioctl(fd, BTRFS_IOC_ADD_DEV, 0);
    if (ret != 0) {
        printf("Failed to add device %s, errno %d\n", path, errno);
        return NULL;
    }
    printf("Device %s added successfully\n", path);
    return NULL;
}



int main(int argc, char **argv) {

    DIR * dir;
    pthread_t th1, th2;
    srand(time(NULL));

    while(1) {
        controller = 0;
        //prepare random delay
        prepare_random();

        dir = opendir(btrfsmntpt);
        if (!dir) {
            printf("Failed to open btrfs mount point %s, errno %d\n", btrfsmntpt, errno);
            return 1;
        }
        fd = dirfd(dir);
        if (fd < 0) {
            printf("Failed to get directory stream file descriptor, errno %d\n", errno);
            return 1;
        }

        pthread_create(&th2, NULL, thr2, NULL);
        pthread_create(&th1, NULL, thr1, NULL);
        // pthread_create(&th2, NULL, closing_thread, &test);

        controller = 1; // start

        pthread_join(th1, NULL);
        pthread_join(th2, NULL);

        closedir(dir);
        close(fd);
    }
    return 0;
}


The procedure to reproduce the problem:
1、mount a device to /mnt/my_btrfs;
2、excute the above compiled repro;


The log of my test:
root@syzkaller:/home/xsk# mount btrfs.img /mnt/my_btrfs/
root@syzkaller:/home/xsk# mount
/dev/sda on / type ext4 (rw,relatime)
....
/home/xsk/btrfs.img on /mnt/my_btrfs type btrfs (rw,relatime,discard=async,space_cache,subvolid=5,subvol=/)
root@syzkaller:/home/xsk# ./repro 
Failed to add device /dev/vda, errno 14
Failed to add device /dev/vda, errno 14
Failed to add device /dev/vda, errno 14
Failed to add device /dev/vda, errno 14
Failed to add device /dev/vda, errno 14
Failed to add de[   69.838916][ T8108] assertion failed: fs_info->exclusive_operation == BTRFS_EXCLOP_BALANCE || fs_info->exclusive_operation == BTRFS_EXCLOP_DEV_ADD, in fs/btrfs/ioctl.c:456
vice /dev/vd[a ,   e69.840759][ T8108] invalid opcode: 0000 [#1] PREEMPT SMP KASAN
9rn[o   1 46
.841461][ T8108] CPU: 0 PID: 8108 Comm: repro Not tainted 6.2.0 #5
[   69.842245][ T8108] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014
[   69.843292][ T8108] RIP: 0010:btrfs_assertfail+0x2c/0x2e
[   69.843899][ T8108] Code: 1f 00 41 55 41 89 d5 41 54 49 89 f4 55 48 89 fd e8 b9 84 e8 f7 44 89 e9 4c 89 e2 48 89 ee 48 c7 c7 20 af 54 8a e8 cb 49 f3 ff <0f> 0b 66 0f 1f 00 55 48 89 fd e8 95 84 e8 f7 48 89 ef 5d 48 c7 c6
[   69.845934][ T8108] RSP: 0018:ffffc90005f1fe28 EFLAGS: 00010282
[   69.846588][ T8108] RAX: 0000000000000097 RBX: 0000000000000000 RCX: 0000000000000000
[   69.847426][ T8108] RDX: ffff88801e8b2440 RSI: ffffffff81637e7c RDI: fffff52000be3fb7
[   69.848267][ T8108] RBP: ffffffff8a533d80 R08: 0000000000000097 R09: 0000000000000000
[   69.849103][ T8108] R10: 0000000080000001 R11: 0000000000000001 R12: ffffffff8a533d40
[   69.849945][ T8108] R13: 00000000000001c8 R14: 000000005000940a R15: 0000000000000000
[   69.850792][ T8108] FS:  00007ff3e2004640(0000) GS:ffff88802cc00000(0000) knlGS:0000000000000000
[   69.851747][ T8108] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   69.852457][ T8108] CR2: 0000000000000000 CR3: 00000000182c8000 CR4: 0000000000750ef0
[   69.853311][ T8108] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[   69.854167][ T8108] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[   69.854935][ T8108] PKRU: 55555554
[   69.855263][ T8108] Call Trace:
[   69.855575][ T8108]  <TASK>
[   69.855851][ T8108]  btrfs_exclop_balance+0x13c/0x310
[   69.856337][ T8108]  ? memdup_user+0xab/0xc0
[   69.856749][ T8108]  ? PTR_ERR+0x17/0x20
[   69.857138][ T8108]  btrfs_ioctl_add_dev+0x2ee/0x320
[   69.857716][ T8108]  btrfs_ioctl+0x9d5/0x10d0
[   69.858222][ T8108]  ? btrfs_ioctl_encoded_write+0xb80/0xb80
[   69.858801][ T8108]  __x64_sys_ioctl+0x197/0x210
[   69.859253][ T8108]  do_syscall_64+0x3c/0xb0
[   69.859668][ T8108]  entry_SYSCALL_64_after_hwframe+0x63/0xcd
[   69.860217][ T8108] RIP: 0033:0x4546af
[   69.860582][ T8108] Code: 00 48 89 44 24 18 31 c0 48 8d 44 24 60 c7 04 24 10 00 00 00 48 89 44 24 08 48 8d 44 24 20 48 89 44 24 10 b8 10 00 00 00 0f 05 <41> 89 c0 3d 00 f0 ff ff 77 1f 48 8b 44 24 18 64 48 2b 04 25 28 00
[   69.862533][ T8108] RSP: 002b:00007ff3e2004150 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
[   69.863299][ T8108] RAX: ffffffffffffffda RBX: 00007ff3e2004640 RCX: 00000000004546af
[   69.864025][ T8108] RDX: 0000000000000000 RSI: 000000005000940a RDI: 0000000000000003
[   69.864739][ T8108] RBP: 00007ff3e20041d0 R08: 0000000000000000 R09: 00007ffd746cf81f
[   69.865528][ T8108] R10: 0000000000000000 R11: 0000000000000246 R12: 00007ff3e2004640
[   69.866375][ T8108] R13: 0000000000000009 R14: 000000000041b320 R15: 00007ff3e1804000
[   69.867115][ T8108]  </TASK>
[   69.867411][ T8108] Modules linked in:
[   69.867888][ T8108] ---[ end trace 0000000000000000 ]---
[   69.868392][ T8108] RIP: 0010:btrfs_assertfail+0x2c/0x2e
[   69.868901][ T8108] Code: 1f 00 41 55 41 89 d5 41 54 49 89 f4 55 48 89 fd e8 b9 84 e8 f7 44 89 e9 4c 89 e2 48 89 ee 48 c7 c7 20 af 54 8a e8 cb 49 f3 ff <0f> 0b 66 0f 1f 00 55 48 89 fd e8 95 84 e8 f7 48 89 ef 5d 48 c7 c6
[   69.870857][ T8108] RSP: 0018:ffffc90005f1fe28 EFLAGS: 00010282
[   69.871422][ T8108] RAX: 0000000000000097 RBX: 0000000000000000 RCX: 0000000000000000
[   69.872148][ T8108] RDX: ffff88801e8b2440 RSI: ffffffff81637e7c RDI: fffff52000be3fb7
[   69.872866][ T8108] RBP: ffffffff8a533d80 R08: 0000000000000097 R09: 0000000000000000
[   69.873681][ T8108] R10: 0000000080000001 R11: 0000000000000001 R12: ffffffff8a533d40
[   69.874530][ T8108] R13: 00000000000001c8 R14: 000000005000940a R15: 0000000000000000
[   69.875270][ T8108] FS:  00007ff3e2004640(0000) GS:ffff88802cc00000(0000) knlGS:0000000000000000
[   69.876084][ T8108] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   69.876681][ T8108] CR2: 0000000000000000 CR3: 00000000182c8000 CR4: 0000000000750ef0
[   69.877464][ T8108] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[   69.878317][ T8108] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[   69.879062][ T8108] PKRU: 55555554
[   69.879400][ T8108] Kernel panic - not syncing: Fatal exception
[   69.880096][ T8108] Kernel Offset: disabled
[   69.880563][ T8108] Rebooting in 86400 seconds..


Please review whether we should patch the assert to add BTRFS_EXCLOP_NONE condtion.
I'll post a patch if needed. thx.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2023-04-06  6:58 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-24  3:16 [PATCH] btrfs: ioctl: fix inaccurate determination of exclusive_operation xiaoshoukui
2023-03-27 23:05 ` David Sterba
     [not found] <20230327230553.GJ10580 () twin ! jikos ! cz>
2023-03-28  9:43 ` xiaoshoukui
2023-03-31 17:45   ` David Sterba
     [not found] <20230331174533.GZ10580 () twin ! jikos ! cz>
2023-04-03  9:37 ` xiaoshoukui
2023-04-04 19:10   ` David Sterba
2023-04-06  6:58     ` xiaoshoukui

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).