[1/2] bcachefs: Fix a potential in the error handling path of use-after-free inbch2_dev_add()

Message ID 3ab17a294fd2b5fcb180d44955b0d76a28af11cb.1694623395.git.christophe.jaillet@wanadoo.fr
State New
Headers
Series [1/2] bcachefs: Fix a potential in the error handling path of use-after-free inbch2_dev_add() |

Commit Message

Christophe JAILLET Sept. 13, 2023, 4:44 p.m. UTC
  If __bch2_dev_attach_bdev() fails, bch2_dev_free() is called twice.
Once here and another time in the error handling path.

This leads to several use-after-free.

Remove the redundant call and only rely on the error handling path.

Fixes: 6a44735653d4 ("bcachefs: Improved superblock-related error messages")
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
 fs/bcachefs/super.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)
  

Comments

Kent Overstreet Sept. 13, 2023, 11:01 p.m. UTC | #1
On Wed, Sep 13, 2023 at 06:44:08PM +0200, Christophe JAILLET wrote:
> If __bch2_dev_attach_bdev() fails, bch2_dev_free() is called twice.
> Once here and another time in the error handling path.
> 
> This leads to several use-after-free.
> 
> Remove the redundant call and only rely on the error handling path.

Thanks, both applied
  
Christophe JAILLET Sept. 14, 2023, 5:28 a.m. UTC | #2
Le 14/09/2023 à 01:01, Kent Overstreet a écrit :
> On Wed, Sep 13, 2023 at 06:44:08PM +0200, Christophe JAILLET wrote:
>> If __bch2_dev_attach_bdev() fails, bch2_dev_free() is called twice.
>> Once here and another time in the error handling path.
>>
>> This leads to several use-after-free.
>>
>> Remove the redundant call and only rely on the error handling path.
> Thanks, both applied



If not too late, it is more a double-free than a use-after-free.

And I messed up the ordering of the words in the subject.

Sorry about that.

CJ
  

Patch

diff --git a/fs/bcachefs/super.c b/fs/bcachefs/super.c
index 29cd71445a94..7379325c428f 100644
--- a/fs/bcachefs/super.c
+++ b/fs/bcachefs/super.c
@@ -1617,10 +1617,8 @@  int bch2_dev_add(struct bch_fs *c, const char *path)
 	bch2_dev_usage_init(ca);
 
 	ret = __bch2_dev_attach_bdev(ca, &sb);
-	if (ret) {
-		bch2_dev_free(ca);
+	if (ret)
 		goto err;
-	}
 
 	ret = bch2_dev_journal_alloc(ca);
 	if (ret) {