btrfs: drop the lock on error in btrfs_dev_replace_cancel()
diff mbox series

Message ID 20190211183209.GA13934@kadam
State New
Headers show
Series
  • btrfs: drop the lock on error in btrfs_dev_replace_cancel()
Related show

Commit Message

Dan Carpenter Feb. 11, 2019, 6:32 p.m. UTC
We should drop the lock on this error path.  This is from static
analysis and I don't know if it's possible to hit this error path in
real life.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
 fs/btrfs/dev-replace.c | 1 +
 1 file changed, 1 insertion(+)

Comments

David Sterba Feb. 19, 2019, 7:04 p.m. UTC | #1
On Mon, Feb 11, 2019 at 09:32:10PM +0300, Dan Carpenter wrote:
> We should drop the lock on this error path.  This is from static
> analysis and I don't know if it's possible to hit this error path in
> real life.

Yes the lock needs to be released, it's there to protect access to the
dev_replace members and is not supposed to be left locked. The value of
state that's being switched would need to be artifically changed to an
invalid value so the default: branch is taken.

It's been introduced by d189dd70e25561817325 in 5.0-rc1 so it counts as
a regression but I don't think it's urgent enough to be sent to a late
rc. It'll go through the stable tree channel. Thanks.
Anand Jain Feb. 20, 2019, 1:20 a.m. UTC | #2
On 2/20/19 3:04 AM, David Sterba wrote:
> On Mon, Feb 11, 2019 at 09:32:10PM +0300, Dan Carpenter wrote:
>> We should drop the lock on this error path.  This is from static
>> analysis and I don't know if it's possible to hit this error path in
>> real life.
> 
> Yes the lock needs to be released, it's there to protect access to the
> dev_replace members and is not supposed to be left locked. The value of
> state that's being switched would need to be artifically changed to an
> invalid value so the default: branch is taken.
> 
> It's been introduced by d189dd70e25561817325 in 5.0-rc1 so it counts as
> a regression but I don't think it's urgent enough to be sent to a late
> rc. It'll go through the stable tree channel. Thanks.
> 

oops I missed this email. Thanks Dan and David.

Reviewed-by: Anand Jain <anand.jain@oracle.com>

Patch
diff mbox series

diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
index 13863354ff9d..ee193c5222b2 100644
--- a/fs/btrfs/dev-replace.c
+++ b/fs/btrfs/dev-replace.c
@@ -862,6 +862,7 @@  int btrfs_dev_replace_cancel(struct btrfs_fs_info *fs_info)
 			btrfs_destroy_dev_replace_tgtdev(tgt_device);
 		break;
 	default:
+		up_write(&dev_replace->rwsem);
 		result = -EINVAL;
 	}