jfs doesn't log block allocations made through ->write_begin() and ->writepages() right away. extAlloc() records them in anonymous transaction locks that hang off the inode and puts the inode on the global anonymous list. They only get committed when a later transaction includes the inode, usually writeback calling ->write_inode() which ends up in jfs_commit_inode(). jfs_evict_inode() relies on that and BUG()s when the inode is still on the anonymous list. But nothing guarantees that writeback ran before eviction. With errors=remount-ro, the default, an error sets SB_RDONLY and sync_filesystem() skips read-only superblocks. So unmounting the filesystem after an error evicts every inode that allocated blocks since it was last written back without committing anything and the BUG_ON() fires. Writing a file and hitting an error before the flusher got to the inode is enough: kernel BUG at fs/jfs/inode.c:175! Oops: invalid opcode: 0000 [#1] SMP KASAN NOPTI RIP: 0010:jfs_evict_inode+0x33c/0x440 Call Trace: evict+0x336/0x8b0 evict_inodes+0x421/0x850 generic_shutdown_super+0xaf/0x530 kill_block_super+0x36/0xa0 deactivate_locked_super+0x97/0xf0 cleanup_mnt+0x9a/0x130 task_work_run+0x135/0x230 exit_to_user_mode_loop+0x171/0x560 do_syscall_64+0x410/0x5e0 entry_SYSCALL_64_after_hwframe+0x77/0x7f Just taking the inode off the anonymous list isn't enough. The tlocks come from TxLock which is shared by all jfs mounts, txLock() pointed mp->lid at them and pinned the metapages with metapage_nohomeok(). Leak them and every other jfs filesystem eventually blocks in txBeginAnon() waiting for tlocks that never come back. So free them the way txUnlock() frees the tlocks of a committed transaction: drop the metapage_nohomeok() pin, reset mp->lid and free the linelocks and the tlock itself. txAbort() isn't a good model here as it only drops the pin for pages that are on the logsync list and leaves the linelocks alone. A metapage that carries nothing but the uncommitted change isn't written home. The superblock is marked dirty after an error so fsck has to look at the filesystem anyway. Note that jfs_sync() takes an inode it can't grab off the anonymous list without freeing anything so check jfs_ip->atlhead instead of the list. Fixes: 8a9cd6d67672 ("JFS: Fix race in txLock") Cc: stable@vger.kernel.org Signed-off-by: Christian Brauner (Amutable) --- fs/jfs/inode.c | 9 +++++++ fs/jfs/jfs_txnmgr.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++++ fs/jfs/jfs_txnmgr.h | 1 + 3 files changed, 78 insertions(+) diff --git a/fs/jfs/inode.c b/fs/jfs/inode.c index c7914dbc91ed9..6390d915c7f11 100644 --- a/fs/jfs/inode.c +++ b/fs/jfs/inode.c @@ -172,6 +172,15 @@ void jfs_evict_inode(struct inode *inode) clear_inode(inode); dquot_drop(inode); + /* + * Writeback commits the anonymous transactions of allocations. If the + * filesystem went read-only without a sync, e.g. after an error, + * nothing did. + */ + if (ji->atlhead) { + WARN_ON_ONCE(!sb_rdonly(inode->i_sb)); + txAbortAnon(inode); + } BUG_ON(!list_empty(&ji->anon_inode_list)); spin_lock_irq(&ji->ag_lock); diff --git a/fs/jfs/jfs_txnmgr.c b/fs/jfs/jfs_txnmgr.c index 083dbbb0c3268..85192dd4461a4 100644 --- a/fs/jfs/jfs_txnmgr.c +++ b/fs/jfs/jfs_txnmgr.c @@ -2581,6 +2581,74 @@ void txFreelock(struct inode *ip) TXN_UNLOCK(); } +/* + * txAbortAnon() + * + * function: free the anonymous tlocks of an inode that is evicted + * before writeback committed them, e.g. because an error made + * the filesystem read-only. + * + * A page that carries nothing but uncommitted changes isn't written home. + * A page that is also waiting to go home with committed changes (mp->lsn) + * goes home with both, after an error the superblock is marked dirty. + */ +void txAbortAnon(struct inode *ip) +{ + struct jfs_inode_info *jfs_ip = JFS_IP(ip); + struct linelock *linelock; + struct metapage *mp; + struct tlock *tlck; + lid_t lid, next, llid, k; + + /* txQuiesce() and jfs_sync() commit under the commit_mutex */ + mutex_lock(&jfs_ip->commit_mutex); + + TXN_LOCK(); + lid = jfs_ip->atlhead; + jfs_ip->atlhead = jfs_ip->atltail = 0; + list_del_init(&jfs_ip->anon_inode_list); + TXN_UNLOCK(); + + for (; lid; lid = next) { + tlck = lid_to_tlock(lid); + next = tlck->next; + mp = tlck->mp; + + if (mp && (tlck->flag & tlckPAGELOCK)) { + /* undo txLock()'s metapage_nohomeok() */ + hold_metapage(mp); + if (!mp->lsn) + clear_bit(META_dirty, &mp->flag); + TXN_LOCK(); + mp->lid = 0; + TXN_UNLOCK(); + _metapage_homeok(mp); + put_metapage(mp); + } else if (mp) { + /* the xtree root in the inode, it isn't written anymore */ + TXN_LOCK(); + if (jfs_ip->xtlid == lid) + jfs_ip->xtlid = 0; + else + mp->lid = 0; + TXN_UNLOCK(); + } + + TXN_LOCK(); + llid = ((struct linelock *) &tlck->lock)->next; + while (llid) { + linelock = (struct linelock *) lid_to_tlock(llid); + k = linelock->next; + txLockFree(llid); + llid = k; + } + txLockFree(lid); + TXN_UNLOCK(); + } + + mutex_unlock(&jfs_ip->commit_mutex); +} + /* * txAbort() * diff --git a/fs/jfs/jfs_txnmgr.h b/fs/jfs/jfs_txnmgr.h index ba71eb5ced567..e9a7547afd3f6 100644 --- a/fs/jfs/jfs_txnmgr.h +++ b/fs/jfs/jfs_txnmgr.h @@ -288,6 +288,7 @@ extern struct linelock *txLinelock(struct linelock *); extern void txFreeMap(struct inode *, struct maplock *, struct tblock *, int); extern void txEA(tid_t, struct inode *, dxd_t *, dxd_t *); extern void txFreelock(struct inode *); +extern void txAbortAnon(struct inode *); extern int lmLog(struct jfs_log *, struct tblock *, struct lrd *, struct tlock *); extern void txQuiesce(struct super_block *); -- 2.53.0