linux-kernel - Re: [GIT PULL] bcachefs changes for 6.11, v2

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 for Android: free password hash cracker in your pocket

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

Message-ID: <ltl35vocjtma5an2yo7digcdpcsvf6clrvcd4vdkf67gwabogf@syqzgnw5rodw>
Date: Tue, 27 Aug 2024 12:23:22 -0400
From: Kent Overstreet <kent.overstreet@...ux.dev>
To: Dan Carpenter <dan.carpenter@...aro.org>
Cc: Linus Torvalds <torvalds@...ux-foundation.org>, 
	linux-bcachefs@...r.kernel.org, linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [GIT PULL] bcachefs changes for 6.11, v2

On Tue, Aug 27, 2024 at 01:53:55PM GMT, Dan Carpenter wrote:
> On Thu, Jul 19, 2024 at 06:36:50PM -0400, Kent Overstreet wrote:
> >       bcachefs: Unlock trans when waiting for user input in fsck
> 
> Hello Kent Overstreet,
> 
> ommit 889fb3dc5d6f ("bcachefs: Unlock trans when waiting for user
> input in fsck") from May 29, 2024 (linux-next), leads to the
> following (UNPUBLISHED) Smatch static checker warning:
> 
> fs/bcachefs/error.c:129 bch2_fsck_ask_yn() error: double unlocked 'trans' (orig line 113)
> 
> fs/bcachefs/error.c
>    102  static enum ask_yn bch2_fsck_ask_yn(struct bch_fs *c, struct btree_trans *trans)
>    103  {
>    104          struct stdio_redirect *stdio = c->stdio;
>    105  
>    106          if (c->stdio_filter && c->stdio_filter != current)
>    107                  stdio = NULL;
>    108  
>    109          if (!stdio)
>    110                  return YN_NO;
>    111  
>    112          if (trans)
>    113                  bch2_trans_unlock(trans);
>                         ^^^^^^^^^^^^^^^^^^^^^^^^^
> Unlock
> 
>    114  
>    115          unsigned long unlock_long_at = trans ? jiffies + HZ * 2 : 0;
>    116          darray_char line = {};
>    117          int ret;
>    118  
>    119          do {
>    120                  unsigned long t;
>    121                  bch2_print(c, " (y,n, or Y,N for all errors of this type) ");
>    122  rewait:
>    123                  t = unlock_long_at
>    124                          ? max_t(long, unlock_long_at - jiffies, 0)
>    125                          : MAX_SCHEDULE_TIMEOUT;
>    126  
>    127                  int r = bch2_stdio_redirect_readline_timeout(stdio, &line, t);
>    128                  if (r == -ETIME) {
>    129                          bch2_trans_unlock_long(trans);
>                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> Double unlock

Those are different types of unlocks.

The first unlock drops btree locks, but we still have pointers and lock
sequence numbers to those nodes so that we can do a bch2_trans_relock()
later, and continue the same transaction.

But we still have an SRCU read lock held which prevents those nodes
from being reclaimed, and we can't hold that for too long either.

So if we're blocked here too long we have to also do an unlock_long(),
which forces a transaction restart.