diff options
author | Eric Van Hensbergen <ericvh@kernel.org> | 2024-01-07 19:03:03 +0000 |
---|---|---|
committer | Eric Van Hensbergen <ericvh@kernel.org> | 2024-01-08 15:12:33 +0000 |
commit | da86de66630466a3644056c97f9caf86b5946e8f (patch) | |
tree | 182453c80057cd9c3480ba2f86956cb6c2b955a4 | |
parent | 9a3c5c5d5d6d8714ea6e3bfbf7eb94743f73ff9b (diff) | |
download | v9fs-b4/fix-nlink-handling.tar.gz |
fs/9p: fix inode nlink accountingb4/fix-nlink-handling
I was running some regressions and noticed a (race-y) kernel warning
that happens when nlink becomes less than zero. Looking through the
code it looks like we aren't good about protecting the inode lock when
manipulating nlink and some code that was added several years ago to
protect against bugs in underlying file systems nlink handling didn't
look quite right either. I took a look at what NFS was doing and tried
to follow similar approaches in the 9p code.
Signed-off-by: Eric Van Hensbergen <ericvh@kernel.org>
-rw-r--r-- | fs/9p/v9fs_vfs.h | 1 | ||||
-rw-r--r-- | fs/9p/vfs_inode.c | 49 | ||||
-rw-r--r-- | fs/9p/vfs_inode_dotl.c | 2 |
3 files changed, 37 insertions, 15 deletions
diff --git a/fs/9p/v9fs_vfs.h b/fs/9p/v9fs_vfs.h index 731e3d14b67d3..6019909445d48 100644 --- a/fs/9p/v9fs_vfs.h +++ b/fs/9p/v9fs_vfs.h @@ -37,6 +37,7 @@ extern const struct file_operations v9fs_dir_operations_dotl; extern const struct dentry_operations v9fs_dentry_operations; extern const struct dentry_operations v9fs_cached_dentry_operations; extern struct kmem_cache *v9fs_inode_cache; +extern void v9fs_inc_nlink(struct inode *inode); struct inode *v9fs_alloc_inode(struct super_block *sb); void v9fs_free_inode(struct inode *inode); diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c index b845ee18a80be..fbd87f43aea2c 100644 --- a/fs/9p/vfs_inode.c +++ b/fs/9p/vfs_inode.c @@ -506,21 +506,43 @@ static int v9fs_at_to_dotl_flags(int flags) } /** - * v9fs_dec_count - helper functon to drop i_nlink. + * v9fs_dec_nlink - helper functon to drop i_nlink. * - * If a directory had nlink <= 2 (including . and ..), then we should not drop - * the link count, which indicates the underlying exported fs doesn't maintain - * nlink accurately. e.g. + * Put a guards around this so we are only dropping nlink + * if it would be valid. This prevents bugs from an underlying + * filesystem implementations from triggering kernel WARNs. We'll + * still print 9p debug messages if the underlying filesystem is wrong. + * + * known underlying filesystems which might exhibit this issue: * - overlayfs sets nlink to 1 for merged dir * - ext4 (with dir_nlink feature enabled) sets nlink to 1 if a dir has more * than EXT4_LINK_MAX (65000) links. * * @inode: inode whose nlink is being dropped */ -static void v9fs_dec_count(struct inode *inode) +static void v9fs_dec_nlink(struct inode *inode) { - if (!S_ISDIR(inode->i_mode) || inode->i_nlink > 2) + spin_lock(&inode->i_lock); + if (inode->i_nlink > 0) drop_nlink(inode); + else + p9_debug(P9_DEBUG_ERROR, "WARNING: nlink is already 0 inode %p\n", + inode); + spin_unlock(&inode->i_lock); +} + +static void v9fs_clear_nlink(struct inode *inode) +{ + spin_lock(&inode->i_lock); + clear_nlink(inode); + spin_unlock(&inode->i_lock); +} + +void v9fs_inc_nlink(struct inode *inode) +{ + spin_lock(&inode->i_lock); + inc_nlink(inode); + spin_unlock(&inode->i_lock); } /** @@ -566,10 +588,9 @@ static int v9fs_remove(struct inode *dir, struct dentry *dentry, int flags) * link count */ if (flags & AT_REMOVEDIR) { - clear_nlink(inode); - v9fs_dec_count(dir); + v9fs_clear_nlink(inode); } else - v9fs_dec_count(inode); + v9fs_dec_nlink(inode); v9fs_invalidate_inode_attr(inode); v9fs_invalidate_inode_attr(dir); @@ -713,7 +734,7 @@ static int v9fs_vfs_mkdir(struct mnt_idmap *idmap, struct inode *dir, err = PTR_ERR(fid); fid = NULL; } else { - inc_nlink(dir); + v9fs_inc_nlink(dir); v9fs_invalidate_inode_attr(dir); } @@ -963,14 +984,14 @@ error_locked: if (!retval) { if (new_inode) { if (S_ISDIR(new_inode->i_mode)) - clear_nlink(new_inode); + v9fs_clear_nlink(new_inode); else - v9fs_dec_count(new_inode); + v9fs_dec_nlink(new_inode); } if (S_ISDIR(old_inode->i_mode)) { if (!new_inode) - inc_nlink(new_dir); - v9fs_dec_count(old_dir); + v9fs_inc_nlink(new_dir); + v9fs_dec_nlink(old_dir); } v9fs_invalidate_inode_attr(old_inode); v9fs_invalidate_inode_attr(old_dir); diff --git a/fs/9p/vfs_inode_dotl.c b/fs/9p/vfs_inode_dotl.c index c7319af2f4711..fe0c41d042a14 100644 --- a/fs/9p/vfs_inode_dotl.c +++ b/fs/9p/vfs_inode_dotl.c @@ -427,7 +427,7 @@ static int v9fs_vfs_mkdir_dotl(struct mnt_idmap *idmap, v9fs_set_create_acl(inode, fid, dacl, pacl); d_instantiate(dentry, inode); } - inc_nlink(dir); + v9fs_inc_nlink(dir); v9fs_invalidate_inode_attr(dir); error: p9_fid_put(fid); |