linux-kernel - Re: [PATCH] bcachefs: don't call sleeping funcs when handling inconsistency errors

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <zjqewsg6ijvol3x3rdjfyjeji3wy24nw45yb6mkqj7vwsk7mrn@yxnvgpj4i24k>
Date: Wed, 2 Apr 2025 12:17:10 -0400
From: Kent Overstreet <kent.overstreet@...ux.dev>
To: Bharadwaj Raju <bharadwaj.raju777@...il.com>
Cc: linux-bcachefs@...r.kernel.org, shuah@...nel.org, 
	linux-kernel@...r.kernel.org, linux-kernel-mentees@...ts.linux.dev, 
	syzbot+c82cd2906e2f192410bb@...kaller.appspotmail.com
Subject: Re: [PATCH] bcachefs: don't call sleeping funcs when handling
 inconsistency errors

On Wed, Apr 02, 2025 at 09:40:40PM +0530, Bharadwaj Raju wrote:
> In bch2_bkey_pick_read_device, we're in an RCU lock. So, we can't call
> any potentially-sleeping functions. However, we call bch2_dev_rcu,
> which calls bch2_fs_inconsistent in its error case. That then calls
> bch2_prt_print on a non-atomic printbuf, as well as uses the blocking
> variant of bch2_print_string_as_lines, both of which lead to calls to
> potentially-sleeping functions, namely krealloc with GFP_KERNEL
> and console_lock respectively.
> 
> Give a nonzero atomic to the printbuf, and use the nonblocking variant
> of bch2_print_string_as_lines.

Sorry, beat you to it :)

You also missed the one the syzbot report actually hit -
bch2_inconsistent_error().

commit fef0ac7dbdd3c2166462720a2c0c9b16ad0680a5
Author: Kent Overstreet <kent.overstreet@...ux.dev>
Date:   Wed Apr 2 11:02:12 2025 -0400

    bcachefs: Fix scheduling while atomic
    
    bch2_inconsistent(), bch2_fs_inconsistent() be called from interrupt
    context, or with rcu_read_lock() held.
    
    The one syzbot found is in
      bch2_bkey_pick_read_device
      bch2_dev_rcu
      bch2_fs_inconsistent
    
    We're starting to switch to lift the printbufs up to higher levels so we
    can emit better log messages and print them all in one go (avoid
    garbling), so that conversion will help with spotting these in the
    future; when we declare a printbuf it must be flagged if we're in an
    atomic context.
    
    Reported-by: syzbot+c82cd2906e2f192410bb@...kaller.appspotmail.com
    Signed-off-by: Kent Overstreet <kent.overstreet@...ux.dev>

diff --git a/fs/bcachefs/error.c b/fs/bcachefs/error.c
index d4dfd13a8076..b885bd92834c 100644
--- a/fs/bcachefs/error.c
+++ b/fs/bcachefs/error.c
@@ -45,6 +45,8 @@ bool __bch2_inconsistent_error(struct bch_fs *c, struct printbuf *out)
 bool bch2_inconsistent_error(struct bch_fs *c)
 {
 	struct printbuf buf = PRINTBUF;
+	buf.atomic++;
+
 	printbuf_indent_add_nextline(&buf, 2);
 
 	bool ret = __bch2_inconsistent_error(c, &buf);
@@ -59,6 +61,7 @@ static bool bch2_fs_trans_inconsistent(struct bch_fs *c, struct btree_trans *tra
 				       const char *fmt, va_list args)
 {
 	struct printbuf buf = PRINTBUF;
+	buf.atomic++;
 
 	bch2_log_msg_start(c, &buf);
 

> 
> Reported-by: syzbot+c82cd2906e2f192410bb@...kaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=c82cd2906e2f192410bb
> Signed-off-by: Bharadwaj Raju <bharadwaj.raju777@...il.com>
> ---
>  fs/bcachefs/error.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/bcachefs/error.c b/fs/bcachefs/error.c
> index d4dfd13a8076..6f0f2f12c004 100644
> --- a/fs/bcachefs/error.c
> +++ b/fs/bcachefs/error.c
> @@ -60,6 +60,8 @@ static bool bch2_fs_trans_inconsistent(struct bch_fs *c, struct btree_trans *tra
>  {
>  	struct printbuf buf = PRINTBUF;
>  
> +	buf.atomic++;
> +
>  	bch2_log_msg_start(c, &buf);
>  
>  	prt_vprintf(&buf, fmt, args);
> @@ -68,7 +70,9 @@ static bool bch2_fs_trans_inconsistent(struct bch_fs *c, struct btree_trans *tra
>  	if (trans)
>  		bch2_trans_updates_to_text(&buf, trans);
>  	bool ret = __bch2_inconsistent_error(c, &buf);
> -	bch2_print_string_as_lines(KERN_ERR, buf.buf);
> +	bch2_print_string_as_lines_nonblocking(KERN_ERR, buf.buf);
> +
> +	buf.atomic--;
>  
>  	printbuf_exit(&buf);
>  	return ret;
> -- 
> 2.49.0
>