[PATCH] bcachefs: fix use-after-free in bch2_dirent_to_text()

Nirbhay Sharma posted 1 patch 2 months, 2 weeks ago
fs/bcachefs/dirent.c | 39 ++++++++++++++++++++++++++++++---------
1 file changed, 30 insertions(+), 9 deletions(-)
[PATCH] bcachefs: fix use-after-free in bch2_dirent_to_text()
Posted by Nirbhay Sharma 2 months, 2 weeks ago
Add bounds checking before calculating pointer offsets in dirent name
accessor functions to prevent out-of-bounds memory access when
processing corrupted filesytem metadata.

When d_name_len contains a corrupted value, the pointer calculation
&d.v->d_cf_name_block.d_names[name_len] results in an offset far
outside the dirent structure, triggering KASAN use-after-free erors.

While bch2_dirent_validate() detects such corruption,
bch2_dirent_to_text() may still be called for debug output, so the
accessor functions must handle invalid data gracefully.

Fixes: c21f41f6905be4fc5059a10a5bba94105ba87269 ("bcachefs: bch2_dirent_to_text() shows casefolded dirents")
Reported-by: syzbot+7f176adb30b21606c5fc@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=7f176adb30b21606c5fc
Signed-off-by: Nirbhay Sharma <nirbhay.lkd@gmail.com>
---
 fs/bcachefs/dirent.c | 39 ++++++++++++++++++++++++++++++---------
 1 file changed, 30 insertions(+), 9 deletions(-)

diff --git a/fs/bcachefs/dirent.c b/fs/bcachefs/dirent.c
index d198001838f3..8be31b41c32b 100644
--- a/fs/bcachefs/dirent.c
+++ b/fs/bcachefs/dirent.c
@@ -58,8 +58,16 @@ static unsigned bch2_dirent_name_bytes(struct bkey_s_c_dirent d)
 
 struct qstr bch2_dirent_get_name(struct bkey_s_c_dirent d)
 {
+	unsigned int name_len, max_len;
+
 	if (d.v->d_casefold) {
-		unsigned name_len = le16_to_cpu(d.v->d_cf_name_block.d_name_len);
+		name_len = le16_to_cpu(d.v->d_cf_name_block.d_name_len);
+		max_len = bkey_val_bytes(d.k) -
+			offsetof(struct bch_dirent, d_cf_name_block.d_names);
+
+		if (name_len > max_len)
+			return (struct qstr) QSTR_INIT(NULL, 0);
+
 		return (struct qstr) QSTR_INIT(&d.v->d_cf_name_block.d_names[0], name_len);
 	} else {
 		return (struct qstr) QSTR_INIT(d.v->d_name, bch2_dirent_name_bytes(d));
@@ -68,13 +76,19 @@ struct qstr bch2_dirent_get_name(struct bkey_s_c_dirent d)
 
 static struct qstr bch2_dirent_get_casefold_name(struct bkey_s_c_dirent d)
 {
-	if (d.v->d_casefold) {
-		unsigned name_len = le16_to_cpu(d.v->d_cf_name_block.d_name_len);
-		unsigned cf_name_len = le16_to_cpu(d.v->d_cf_name_block.d_cf_name_len);
-		return (struct qstr) QSTR_INIT(&d.v->d_cf_name_block.d_names[name_len], cf_name_len);
-	} else {
+	unsigned int name_len, cf_name_len, max_len;
+
+	if (!d.v->d_casefold)
 		return (struct qstr) QSTR_INIT(NULL, 0);
-	}
+
+	name_len = le16_to_cpu(d.v->d_cf_name_block.d_name_len);
+	cf_name_len = le16_to_cpu(d.v->d_cf_name_block.d_cf_name_len);
+	max_len = bkey_val_bytes(d.k) - offsetof(struct bch_dirent, d_cf_name_block.d_names);
+
+	if (name_len > max_len || cf_name_len > max_len || name_len + cf_name_len > max_len)
+		return (struct qstr) QSTR_INIT(NULL, 0);
+
+	return (struct qstr) QSTR_INIT(&d.v->d_cf_name_block.d_names[name_len], cf_name_len);
 }
 
 static inline struct qstr bch2_dirent_get_lookup_name(struct bkey_s_c_dirent d)
@@ -212,11 +226,18 @@ void bch2_dirent_to_text(struct printbuf *out, struct bch_fs *c, struct bkey_s_c
 	struct bkey_s_c_dirent d = bkey_s_c_to_dirent(k);
 	struct qstr d_name = bch2_dirent_get_name(d);
 
+	if (!d_name.name || !d_name.len) {
+		prt_str(out, "(invalid)");
+		return;
+	}
+
 	prt_printf(out, "%.*s", d_name.len, d_name.name);
 
 	if (d.v->d_casefold) {
-		struct qstr d_name = bch2_dirent_get_lookup_name(d);
-		prt_printf(out, " (casefold %.*s)", d_name.len, d_name.name);
+		struct qstr d_cf_name = bch2_dirent_get_lookup_name(d);
+
+		if (d_cf_name.name && d_cf_name.len)
+			prt_printf(out, " (casefold %.*s)", d_name.len, d_name.name);
 	}
 
 	prt_str(out, " ->");
-- 
2.51.0
Re: [PATCH] bcachefs: fix use-after-free in bch2_dirent_to_text()
Posted by Kent Overstreet 2 months, 1 week ago
On Fri, Oct 03, 2025 at 04:40:33AM +0530, Nirbhay Sharma wrote:
> Add bounds checking before calculating pointer offsets in dirent name
> accessor functions to prevent out-of-bounds memory access when
> processing corrupted filesytem metadata.
> 
> When d_name_len contains a corrupted value, the pointer calculation
> &d.v->d_cf_name_block.d_names[name_len] results in an offset far
> outside the dirent structure, triggering KASAN use-after-free erors.

We validate d_name_len in bch2_dirent_validate(), so... this looks
entirely bogus.

Did you test any of this?

> 
> While bch2_dirent_validate() detects such corruption,
> bch2_dirent_to_text() may still be called for debug output, so the
> accessor functions must handle invalid data gracefully.
> 
> Fixes: c21f41f6905be4fc5059a10a5bba94105ba87269 ("bcachefs: bch2_dirent_to_text() shows casefolded dirents")
> Reported-by: syzbot+7f176adb30b21606c5fc@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=7f176adb30b21606c5fc
> Signed-off-by: Nirbhay Sharma <nirbhay.lkd@gmail.com>
> ---
>  fs/bcachefs/dirent.c | 39 ++++++++++++++++++++++++++++++---------
>  1 file changed, 30 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/bcachefs/dirent.c b/fs/bcachefs/dirent.c
> index d198001838f3..8be31b41c32b 100644
> --- a/fs/bcachefs/dirent.c
> +++ b/fs/bcachefs/dirent.c
> @@ -58,8 +58,16 @@ static unsigned bch2_dirent_name_bytes(struct bkey_s_c_dirent d)
>  
>  struct qstr bch2_dirent_get_name(struct bkey_s_c_dirent d)
>  {
> +	unsigned int name_len, max_len;
> +
>  	if (d.v->d_casefold) {
> -		unsigned name_len = le16_to_cpu(d.v->d_cf_name_block.d_name_len);
> +		name_len = le16_to_cpu(d.v->d_cf_name_block.d_name_len);
> +		max_len = bkey_val_bytes(d.k) -
> +			offsetof(struct bch_dirent, d_cf_name_block.d_names);
> +
> +		if (name_len > max_len)
> +			return (struct qstr) QSTR_INIT(NULL, 0);
> +
>  		return (struct qstr) QSTR_INIT(&d.v->d_cf_name_block.d_names[0], name_len);
>  	} else {
>  		return (struct qstr) QSTR_INIT(d.v->d_name, bch2_dirent_name_bytes(d));
> @@ -68,13 +76,19 @@ struct qstr bch2_dirent_get_name(struct bkey_s_c_dirent d)
>  
>  static struct qstr bch2_dirent_get_casefold_name(struct bkey_s_c_dirent d)
>  {
> -	if (d.v->d_casefold) {
> -		unsigned name_len = le16_to_cpu(d.v->d_cf_name_block.d_name_len);
> -		unsigned cf_name_len = le16_to_cpu(d.v->d_cf_name_block.d_cf_name_len);
> -		return (struct qstr) QSTR_INIT(&d.v->d_cf_name_block.d_names[name_len], cf_name_len);
> -	} else {
> +	unsigned int name_len, cf_name_len, max_len;
> +
> +	if (!d.v->d_casefold)
>  		return (struct qstr) QSTR_INIT(NULL, 0);
> -	}
> +
> +	name_len = le16_to_cpu(d.v->d_cf_name_block.d_name_len);
> +	cf_name_len = le16_to_cpu(d.v->d_cf_name_block.d_cf_name_len);
> +	max_len = bkey_val_bytes(d.k) - offsetof(struct bch_dirent, d_cf_name_block.d_names);
> +
> +	if (name_len > max_len || cf_name_len > max_len || name_len + cf_name_len > max_len)
> +		return (struct qstr) QSTR_INIT(NULL, 0);
> +
> +	return (struct qstr) QSTR_INIT(&d.v->d_cf_name_block.d_names[name_len], cf_name_len);
>  }
>  
>  static inline struct qstr bch2_dirent_get_lookup_name(struct bkey_s_c_dirent d)
> @@ -212,11 +226,18 @@ void bch2_dirent_to_text(struct printbuf *out, struct bch_fs *c, struct bkey_s_c
>  	struct bkey_s_c_dirent d = bkey_s_c_to_dirent(k);
>  	struct qstr d_name = bch2_dirent_get_name(d);
>  
> +	if (!d_name.name || !d_name.len) {
> +		prt_str(out, "(invalid)");
> +		return;
> +	}
> +
>  	prt_printf(out, "%.*s", d_name.len, d_name.name);
>  
>  	if (d.v->d_casefold) {
> -		struct qstr d_name = bch2_dirent_get_lookup_name(d);
> -		prt_printf(out, " (casefold %.*s)", d_name.len, d_name.name);
> +		struct qstr d_cf_name = bch2_dirent_get_lookup_name(d);
> +
> +		if (d_cf_name.name && d_cf_name.len)
> +			prt_printf(out, " (casefold %.*s)", d_name.len, d_name.name);
>  	}
>  
>  	prt_str(out, " ->");
> -- 
> 2.51.0
>