fuse_do_statx() was added with the wrong helper.
Fixes: d3045530bdd2 ("fuse: implement statx") Cc: stable@vger.kernel.org # v6.6 Signed-off-by: Miklos Szeredi mszeredi@redhat.com --- fs/fuse/dir.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c index 25c7c97f774b..ce6a38c56d54 100644 --- a/fs/fuse/dir.c +++ b/fs/fuse/dir.c @@ -1214,7 +1214,7 @@ static int fuse_do_statx(struct inode *inode, struct file *file, if (((sx->mask & STATX_SIZE) && !fuse_valid_size(sx->size)) || ((sx->mask & STATX_TYPE) && (!fuse_valid_type(sx->mode) || inode_wrong_type(inode, sx->mode)))) { - make_bad_inode(inode); + fuse_make_bad(inode); return -EIO; }
The root inode has a fixed nodeid and generation (1, 0).
Prior to the commit 15db16837a35 ("fuse: fix illegal access to inode with reused nodeid") generation number on lookup was ignored. After this commit lookup with the wrong generation number resulted in the inode being unhashed. This is correct for non-root inodes, but replacing the root inode is wrong and results in weird behavior.
Fix by reverting to the old behavior if ignoring the generation for the root inode, but issuing a warning in dmesg.
Reported-by: Antonio SJ Musumeci trapexit@spawn.link Closes: https://lore.kernel.org/all/CAOQ4uxhek5ytdN8Yz2tNEOg5ea4NkBb4nk0FGPjPk_9nz-V... Fixes: 15db16837a35 ("fuse: fix illegal access to inode with reused nodeid") Cc: stable@vger.kernel.org # v5.14 Signed-off-by: Miklos Szeredi mszeredi@redhat.com --- fs/fuse/dir.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c index ce6a38c56d54..befb7dfe387a 100644 --- a/fs/fuse/dir.c +++ b/fs/fuse/dir.c @@ -391,6 +391,10 @@ int fuse_lookup_name(struct super_block *sb, u64 nodeid, const struct qstr *name err = -EIO; if (fuse_invalid_attr(&outarg->attr)) goto out_put_forget; + if (outarg->nodeid == FUSE_ROOT_ID && outarg->generation != 0) { + pr_warn_once("root generation should be zero\n"); + outarg->generation = 0; + }
*inode = fuse_iget(sb, outarg->nodeid, outarg->generation, &outarg->attr, ATTR_TIMEOUT(outarg),
The root inode is assumed to be always hashed. Do not unhash the root inode even if it is marked BAD.
Fixes: 5d069dbe8aaf ("fuse: fix bad inode") Cc: stable@vger.kernel.org # v5.11 Signed-off-by: Miklos Szeredi mszeredi@redhat.com --- fs/fuse/fuse_i.h | 1 - fs/fuse/inode.c | 7 +++++-- 2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h index 7bd3552b1e80..4ef6087f0e5c 100644 --- a/fs/fuse/fuse_i.h +++ b/fs/fuse/fuse_i.h @@ -994,7 +994,6 @@ static inline bool fuse_stale_inode(const struct inode *inode, int generation,
static inline void fuse_make_bad(struct inode *inode) { - remove_inode_hash(inode); set_bit(FUSE_I_BAD, &get_fuse_inode(inode)->state); }
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c index c26a84439934..aa0614e8791c 100644 --- a/fs/fuse/inode.c +++ b/fs/fuse/inode.c @@ -475,8 +475,11 @@ struct inode *fuse_iget(struct super_block *sb, u64 nodeid, } else if (fuse_stale_inode(inode, generation, attr)) { /* nodeid was reused, any I/O on the old inode should fail */ fuse_make_bad(inode); - iput(inode); - goto retry; + if (inode != d_inode(sb->s_root)) { + remove_inode_hash(inode); + iput(inode); + goto retry; + } } fi = get_fuse_inode(inode); spin_lock(&fi->lock);
On 2/28/24 17:02, Miklos Szeredi wrote:
The root inode is assumed to be always hashed. Do not unhash the root inode even if it is marked BAD.
Fixes: 5d069dbe8aaf ("fuse: fix bad inode") Cc: stable@vger.kernel.org # v5.11 Signed-off-by: Miklos Szeredi mszeredi@redhat.com
fs/fuse/fuse_i.h | 1 - fs/fuse/inode.c | 7 +++++-- 2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h index 7bd3552b1e80..4ef6087f0e5c 100644 --- a/fs/fuse/fuse_i.h +++ b/fs/fuse/fuse_i.h @@ -994,7 +994,6 @@ static inline bool fuse_stale_inode(const struct inode *inode, int generation, static inline void fuse_make_bad(struct inode *inode) {
- remove_inode_hash(inode); set_bit(FUSE_I_BAD, &get_fuse_inode(inode)->state);
}
Hmm, what about callers like fuse_direntplus_link? It now never removes the inode hash for these? Depend on lookup/revalidate?
Thanks, Bernd
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c index c26a84439934..aa0614e8791c 100644 --- a/fs/fuse/inode.c +++ b/fs/fuse/inode.c @@ -475,8 +475,11 @@ struct inode *fuse_iget(struct super_block *sb, u64 nodeid, } else if (fuse_stale_inode(inode, generation, attr)) { /* nodeid was reused, any I/O on the old inode should fail */ fuse_make_bad(inode);
iput(inode);goto retry;
if (inode != d_inode(sb->s_root)) {remove_inode_hash(inode);iput(inode);goto retry;}
On Wed, 28 Feb 2024 at 17:34, Bernd Schubert bernd.schubert@fastmail.fm wrote:
On 2/28/24 17:02, Miklos Szeredi wrote:
The root inode is assumed to be always hashed. Do not unhash the root inode even if it is marked BAD.
Fixes: 5d069dbe8aaf ("fuse: fix bad inode") Cc: stable@vger.kernel.org # v5.11 Signed-off-by: Miklos Szeredi mszeredi@redhat.com
fs/fuse/fuse_i.h | 1 - fs/fuse/inode.c | 7 +++++-- 2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h index 7bd3552b1e80..4ef6087f0e5c 100644 --- a/fs/fuse/fuse_i.h +++ b/fs/fuse/fuse_i.h @@ -994,7 +994,6 @@ static inline bool fuse_stale_inode(const struct inode *inode, int generation,
static inline void fuse_make_bad(struct inode *inode) {
remove_inode_hash(inode); set_bit(FUSE_I_BAD, &get_fuse_inode(inode)->state);}
Hmm, what about callers like fuse_direntplus_link? It now never removes the inode hash for these? Depend on lookup/revalidate?
Good questions.
In that case the dentry will be unhashed, and after retrying it will go through fuse_iget(), which will unhash the inode.
So AFAICS the only place the inode needs to be unhashed is in fuse_iget(), which is the real fix in 775c5033a0d1 ("fuse: fix live lock in fuse_iget()").
Thanks, Miklos
linux-stable-mirror@lists.linaro.org
-
Bernd Schubert -
Miklos Szeredi -
Miklos Szeredi