On Mon, Mar 27, 2023 at 07:24:25AM +0900, Tetsuo Handa wrote: > syzbot is reporting that __getblk_gfp() cannot be called from atomic > context. Fix this problem by converting pointers_lock from rw_lock to > rw_sem. > > Reported-by: syzbot <syzbot+69b40dc5fd40f32c199f@xxxxxxxxxxxxxxxxxxxxxxxxx> > Link: https://syzkaller.appspot.com/bug?extid=69b40dc5fd40f32c199f > Signed-off-by: Tetsuo Handa <penguin-kernel@xxxxxxxxxxxxxxxxxxx> > Tested-by: syzbot <syzbot+69b40dc5fd40f32c199f@xxxxxxxxxxxxxxxxxxxxxxxxx> Hmm... The bug is real, all right (introduced back in 2002 during the conversion away from BKL; commit 3ba77f860fa7f359660e9d498a5b35940021cfba Author: Christoph Hellwig <hch@xxxxxxxxxxxxxxxx> Date: Thu Apr 4 00:25:37 2002 +0200 Replace BKL for chain locking with sysvfs-private rwlock. is where it had happened). However, I don't think this is the right fix. Note that the problem is with get_branch() done under the rwlock; all other places are safe. But in get_branch() we only need the lock (and only shared, at that) around the verify+add pair. See how it's done in fs/minix/itree_common.c... Something like this: diff --git a/fs/sysv/itree.c b/fs/sysv/itree.c index b22764fe669c..cfa281fb6578 100644 --- a/fs/sysv/itree.c +++ b/fs/sysv/itree.c @@ -104,15 +104,18 @@ static Indirect *get_branch(struct inode *inode, bh = sb_bread(sb, block); if (!bh) goto failure; + read_lock(&pointers_lock); if (!verify_chain(chain, p)) goto changed; add_chain(++p, bh, (sysv_zone_t*)bh->b_data + *++offsets); + read_unlock(&pointers_lock); if (!p->key) goto no_block; } return NULL; changed: + read_unlock(&pointers_lock); brelse(bh); *err = -EAGAIN; goto no_block; @@ -214,9 +217,7 @@ static int get_block(struct inode *inode, sector_t iblock, struct buffer_head *b goto out; reread: - read_lock(&pointers_lock); partial = get_branch(inode, depth, offsets, chain, &err); - read_unlock(&pointers_lock); /* Simplest case - block found, no allocation needed */ if (!partial) { @@ -287,10 +288,11 @@ static Indirect *find_shared(struct inode *inode, for (k = depth; k > 1 && !offsets[k-1]; k--) ; - write_lock(&pointers_lock); partial = get_branch(inode, k, offsets, chain, &err); if (!partial) partial = chain + k-1; + + write_lock(&pointers_lock); /* * If the branch acquired continuation since we've looked at it - * fine, it should all survive and (new) top doesn't belong to us.
