Skip to content

Commit

Permalink
Merge tag '9p-for-6.12-rc5' of https://github.com/martinetd/linux
Browse files Browse the repository at this point in the history
Pull more 9p reverts from Dominique Martinet:
 "Revert patches causing inode collision problems.

  The code simplification introduced significant regressions on servers
  that do not remap inode numbers when exporting multiple underlying
  filesystems with colliding inodes. See the top-most revert (commit
  be2ca38) for details.

  This problem had been ignored for too long and the reverts will also
  head to stable (6.9+).

  I'm confident this set of patches gets us back to previous behaviour
  (another related patch had already been reverted back in April and
  we're almost back to square 1, and the rest didn't touch inode
  lifecycle)"

* tag '9p-for-6.12-rc5' of https://github.com/martinetd/linux:
  Revert "fs/9p: simplify iget to remove unnecessary paths"
  Revert "fs/9p: fix uaf in in v9fs_stat2inode_dotl"
  Revert "fs/9p: remove redundant pointer v9ses"
  Revert " fs/9p: mitigate inode collisions"
  • Loading branch information
torvalds committed Oct 25, 2024
2 parents c71f8fb + be2ca38 commit 850925a
Show file tree
Hide file tree
Showing 5 changed files with 192 additions and 87 deletions.
34 changes: 27 additions & 7 deletions fs/9p/v9fs.h
Original file line number Diff line number Diff line change
Expand Up @@ -179,14 +179,16 @@ extern int v9fs_vfs_rename(struct mnt_idmap *idmap,
struct inode *old_dir, struct dentry *old_dentry,
struct inode *new_dir, struct dentry *new_dentry,
unsigned int flags);
extern struct inode *v9fs_fid_iget(struct super_block *sb, struct p9_fid *fid,
bool new);
extern struct inode *v9fs_inode_from_fid(struct v9fs_session_info *v9ses,
struct p9_fid *fid,
struct super_block *sb, int new);
extern const struct inode_operations v9fs_dir_inode_operations_dotl;
extern const struct inode_operations v9fs_file_inode_operations_dotl;
extern const struct inode_operations v9fs_symlink_inode_operations_dotl;
extern const struct netfs_request_ops v9fs_req_ops;
extern struct inode *v9fs_fid_iget_dotl(struct super_block *sb,
struct p9_fid *fid, bool new);
extern struct inode *v9fs_inode_from_fid_dotl(struct v9fs_session_info *v9ses,
struct p9_fid *fid,
struct super_block *sb, int new);

/* other default globals */
#define V9FS_PORT 564
Expand Down Expand Up @@ -225,12 +227,30 @@ static inline int v9fs_proto_dotl(struct v9fs_session_info *v9ses)
*/
static inline struct inode *
v9fs_get_inode_from_fid(struct v9fs_session_info *v9ses, struct p9_fid *fid,
struct super_block *sb, bool new)
struct super_block *sb)
{
if (v9fs_proto_dotl(v9ses))
return v9fs_fid_iget_dotl(sb, fid, new);
return v9fs_inode_from_fid_dotl(v9ses, fid, sb, 0);
else
return v9fs_fid_iget(sb, fid, new);
return v9fs_inode_from_fid(v9ses, fid, sb, 0);
}

/**
* v9fs_get_new_inode_from_fid - Helper routine to populate an inode by
* issuing a attribute request
* @v9ses: session information
* @fid: fid to issue attribute request for
* @sb: superblock on which to create inode
*
*/
static inline struct inode *
v9fs_get_new_inode_from_fid(struct v9fs_session_info *v9ses, struct p9_fid *fid,
struct super_block *sb)
{
if (v9fs_proto_dotl(v9ses))
return v9fs_inode_from_fid_dotl(v9ses, fid, sb, 1);
else
return v9fs_inode_from_fid(v9ses, fid, sb, 1);
}

#endif
2 changes: 1 addition & 1 deletion fs/9p/v9fs_vfs.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ struct inode *v9fs_alloc_inode(struct super_block *sb);
void v9fs_free_inode(struct inode *inode);
void v9fs_set_netfs_context(struct inode *inode);
int v9fs_init_inode(struct v9fs_session_info *v9ses,
struct inode *inode, struct p9_qid *qid, umode_t mode, dev_t rdev);
struct inode *inode, umode_t mode, dev_t rdev);
void v9fs_evict_inode(struct inode *inode);
#if (BITS_PER_LONG == 32)
#define QID2INO(q) ((ino_t) (((q)->path+2) ^ (((q)->path) >> 32)))
Expand Down
129 changes: 82 additions & 47 deletions fs/9p/vfs_inode.c
Original file line number Diff line number Diff line change
Expand Up @@ -256,12 +256,9 @@ void v9fs_set_netfs_context(struct inode *inode)
}

int v9fs_init_inode(struct v9fs_session_info *v9ses,
struct inode *inode, struct p9_qid *qid, umode_t mode, dev_t rdev)
struct inode *inode, umode_t mode, dev_t rdev)
{
int err = 0;
struct v9fs_inode *v9inode = V9FS_I(inode);

memcpy(&v9inode->qid, qid, sizeof(struct p9_qid));

inode_init_owner(&nop_mnt_idmap, inode, NULL, mode);
inode->i_blocks = 0;
Expand Down Expand Up @@ -365,59 +362,105 @@ void v9fs_evict_inode(struct inode *inode)
clear_inode(inode);
}

struct inode *
v9fs_fid_iget(struct super_block *sb, struct p9_fid *fid, bool new)
static int v9fs_test_inode(struct inode *inode, void *data)
{
int umode;
dev_t rdev;
struct v9fs_inode *v9inode = V9FS_I(inode);
struct p9_wstat *st = (struct p9_wstat *)data;
struct v9fs_session_info *v9ses = v9fs_inode2v9ses(inode);

umode = p9mode2unixmode(v9ses, st, &rdev);
/* don't match inode of different type */
if (inode_wrong_type(inode, umode))
return 0;

/* compare qid details */
if (memcmp(&v9inode->qid.version,
&st->qid.version, sizeof(v9inode->qid.version)))
return 0;

if (v9inode->qid.type != st->qid.type)
return 0;

if (v9inode->qid.path != st->qid.path)
return 0;
return 1;
}

static int v9fs_test_new_inode(struct inode *inode, void *data)
{
return 0;
}

static int v9fs_set_inode(struct inode *inode, void *data)
{
struct v9fs_inode *v9inode = V9FS_I(inode);
struct p9_wstat *st = (struct p9_wstat *)data;

memcpy(&v9inode->qid, &st->qid, sizeof(st->qid));
return 0;
}

static struct inode *v9fs_qid_iget(struct super_block *sb,
struct p9_qid *qid,
struct p9_wstat *st,
int new)
{
dev_t rdev;
int retval;
umode_t umode;
struct inode *inode;
struct p9_wstat *st;
struct v9fs_session_info *v9ses = sb->s_fs_info;
int (*test)(struct inode *inode, void *data);

inode = iget_locked(sb, QID2INO(&fid->qid));
if (unlikely(!inode))
return ERR_PTR(-ENOMEM);
if (!(inode->i_state & I_NEW)) {
if (!new) {
goto done;
} else {
p9_debug(P9_DEBUG_VFS, "WARNING: Inode collision %ld\n",
inode->i_ino);
iput(inode);
remove_inode_hash(inode);
inode = iget_locked(sb, QID2INO(&fid->qid));
WARN_ON(!(inode->i_state & I_NEW));
}
}
if (new)
test = v9fs_test_new_inode;
else
test = v9fs_test_inode;

inode = iget5_locked(sb, QID2INO(qid), test, v9fs_set_inode, st);
if (!inode)
return ERR_PTR(-ENOMEM);
if (!(inode->i_state & I_NEW))
return inode;
/*
* initialize the inode with the stat info
* FIXME!! we may need support for stale inodes
* later.
*/
st = p9_client_stat(fid);
if (IS_ERR(st)) {
retval = PTR_ERR(st);
goto error;
}

inode->i_ino = QID2INO(qid);
umode = p9mode2unixmode(v9ses, st, &rdev);
retval = v9fs_init_inode(v9ses, inode, &fid->qid, umode, rdev);
v9fs_stat2inode(st, inode, sb, 0);
p9stat_free(st);
kfree(st);
retval = v9fs_init_inode(v9ses, inode, umode, rdev);
if (retval)
goto error;

v9fs_stat2inode(st, inode, sb, 0);
v9fs_set_netfs_context(inode);
v9fs_cache_inode_get_cookie(inode);
unlock_new_inode(inode);
done:
return inode;
error:
iget_failed(inode);
return ERR_PTR(retval);

}

struct inode *
v9fs_inode_from_fid(struct v9fs_session_info *v9ses, struct p9_fid *fid,
struct super_block *sb, int new)
{
struct p9_wstat *st;
struct inode *inode = NULL;

st = p9_client_stat(fid);
if (IS_ERR(st))
return ERR_CAST(st);

inode = v9fs_qid_iget(sb, &st->qid, st, new);
p9stat_free(st);
kfree(st);
return inode;
}

/**
Expand Down Expand Up @@ -449,15 +492,8 @@ static int v9fs_at_to_dotl_flags(int flags)
*/
static void v9fs_dec_count(struct inode *inode)
{
if (!S_ISDIR(inode->i_mode) || inode->i_nlink > 2) {
if (inode->i_nlink) {
drop_nlink(inode);
} else {
p9_debug(P9_DEBUG_VFS,
"WARNING: unexpected i_nlink zero %d inode %ld\n",
inode->i_nlink, inode->i_ino);
}
}
if (!S_ISDIR(inode->i_mode) || inode->i_nlink > 2)
drop_nlink(inode);
}

/**
Expand Down Expand Up @@ -508,9 +544,6 @@ static int v9fs_remove(struct inode *dir, struct dentry *dentry, int flags)
} else
v9fs_dec_count(inode);

if (inode->i_nlink <= 0) /* no more refs unhash it */
remove_inode_hash(inode);

v9fs_invalidate_inode_attr(inode);
v9fs_invalidate_inode_attr(dir);

Expand Down Expand Up @@ -576,7 +609,7 @@ v9fs_create(struct v9fs_session_info *v9ses, struct inode *dir,
/*
* instantiate inode and assign the unopened fid to the dentry
*/
inode = v9fs_get_inode_from_fid(v9ses, fid, dir->i_sb, true);
inode = v9fs_get_new_inode_from_fid(v9ses, fid, dir->i_sb);
if (IS_ERR(inode)) {
err = PTR_ERR(inode);
p9_debug(P9_DEBUG_VFS,
Expand Down Expand Up @@ -704,8 +737,10 @@ struct dentry *v9fs_vfs_lookup(struct inode *dir, struct dentry *dentry,
inode = NULL;
else if (IS_ERR(fid))
inode = ERR_CAST(fid);
else if (v9ses->cache & (CACHE_META|CACHE_LOOSE))
inode = v9fs_get_inode_from_fid(v9ses, fid, dir->i_sb);
else
inode = v9fs_get_inode_from_fid(v9ses, fid, dir->i_sb, false);
inode = v9fs_get_new_inode_from_fid(v9ses, fid, dir->i_sb);
/*
* If we had a rename on the server and a parallel lookup
* for the new name, then make sure we instantiate with
Expand Down
Loading

0 comments on commit 850925a

Please sign in to comment.