Commit 5d069dbe authored by Miklos Szeredi's avatar Miklos Szeredi
Browse files

fuse: fix bad inode



Jan Kara's analysis of the syzbot report (edited):

  The reproducer opens a directory on FUSE filesystem, it then attaches
  dnotify mark to the open directory.  After that a fuse_do_getattr() call
  finds that attributes returned by the server are inconsistent, and calls
  make_bad_inode() which, among other things does:

          inode->i_mode = S_IFREG;

  This then confuses dnotify which doesn't tear down its structures
  properly and eventually crashes.

Avoid calling make_bad_inode() on a live inode: switch to a private flag on
the fuse inode.  Also add the test to ops which the bad_inode_ops would
have caught.

This bug goes back to the initial merge of fuse in 2.6.14...

Reported-by: default avatar <syzbot+f427adf9324b92652ccc@syzkaller.appspotmail.com>
Signed-off-by: default avatarMiklos Szeredi <mszeredi@redhat.com>
Tested-by: default avatarJan Kara <jack@suse.cz>
Cc: <stable@vger.kernel.org>
parent 9d769e6a
Loading
Loading
Loading
Loading
+6 −0
Original line number Diff line number Diff line
@@ -19,6 +19,9 @@ struct posix_acl *fuse_get_acl(struct inode *inode, int type)
	void *value = NULL;
	struct posix_acl *acl;

	if (fuse_is_bad(inode))
		return ERR_PTR(-EIO);

	if (!fc->posix_acl || fc->no_getxattr)
		return NULL;

@@ -53,6 +56,9 @@ int fuse_set_acl(struct inode *inode, struct posix_acl *acl, int type)
	const char *name;
	int ret;

	if (fuse_is_bad(inode))
		return -EIO;

	if (!fc->posix_acl || fc->no_setxattr)
		return -EOPNOTSUPP;

+32 −5
Original line number Diff line number Diff line
@@ -202,7 +202,7 @@ static int fuse_dentry_revalidate(struct dentry *entry, unsigned int flags)
	int ret;

	inode = d_inode_rcu(entry);
	if (inode && is_bad_inode(inode))
	if (inode && fuse_is_bad(inode))
		goto invalid;
	else if (time_before64(fuse_dentry_time(entry), get_jiffies_64()) ||
		 (flags & (LOOKUP_EXCL | LOOKUP_REVAL))) {
@@ -462,6 +462,9 @@ static struct dentry *fuse_lookup(struct inode *dir, struct dentry *entry,
	bool outarg_valid = true;
	bool locked;

	if (fuse_is_bad(dir))
		return ERR_PTR(-EIO);

	locked = fuse_lock_inode(dir);
	err = fuse_lookup_name(dir->i_sb, get_node_id(dir), &entry->d_name,
			       &outarg, &inode);
@@ -611,6 +614,9 @@ static int fuse_atomic_open(struct inode *dir, struct dentry *entry,
	struct fuse_conn *fc = get_fuse_conn(dir);
	struct dentry *res = NULL;

	if (fuse_is_bad(dir))
		return -EIO;

	if (d_in_lookup(entry)) {
		res = fuse_lookup(dir, entry, 0);
		if (IS_ERR(res))
@@ -659,6 +665,9 @@ static int create_new_entry(struct fuse_mount *fm, struct fuse_args *args,
	int err;
	struct fuse_forget_link *forget;

	if (fuse_is_bad(dir))
		return -EIO;

	forget = fuse_alloc_forget();
	if (!forget)
		return -ENOMEM;
@@ -786,6 +795,9 @@ static int fuse_unlink(struct inode *dir, struct dentry *entry)
	struct fuse_mount *fm = get_fuse_mount(dir);
	FUSE_ARGS(args);

	if (fuse_is_bad(dir))
		return -EIO;

	args.opcode = FUSE_UNLINK;
	args.nodeid = get_node_id(dir);
	args.in_numargs = 1;
@@ -822,6 +834,9 @@ static int fuse_rmdir(struct inode *dir, struct dentry *entry)
	struct fuse_mount *fm = get_fuse_mount(dir);
	FUSE_ARGS(args);

	if (fuse_is_bad(dir))
		return -EIO;

	args.opcode = FUSE_RMDIR;
	args.nodeid = get_node_id(dir);
	args.in_numargs = 1;
@@ -900,6 +915,9 @@ static int fuse_rename2(struct inode *olddir, struct dentry *oldent,
	struct fuse_conn *fc = get_fuse_conn(olddir);
	int err;

	if (fuse_is_bad(olddir))
		return -EIO;

	if (flags & ~(RENAME_NOREPLACE | RENAME_EXCHANGE | RENAME_WHITEOUT))
		return -EINVAL;

@@ -1035,7 +1053,7 @@ static int fuse_do_getattr(struct inode *inode, struct kstat *stat,
	if (!err) {
		if (fuse_invalid_attr(&outarg.attr) ||
		    (inode->i_mode ^ outarg.attr.mode) & S_IFMT) {
			make_bad_inode(inode);
			fuse_make_bad(inode);
			err = -EIO;
		} else {
			fuse_change_attributes(inode, &outarg.attr,
@@ -1237,6 +1255,9 @@ static int fuse_permission(struct inode *inode, int mask)
	bool refreshed = false;
	int err = 0;

	if (fuse_is_bad(inode))
		return -EIO;

	if (!fuse_allow_current_process(fc))
		return -EACCES;

@@ -1332,7 +1353,7 @@ static const char *fuse_get_link(struct dentry *dentry, struct inode *inode,
	int err;

	err = -EIO;
	if (is_bad_inode(inode))
	if (fuse_is_bad(inode))
		goto out_err;

	if (fc->cache_symlinks)
@@ -1380,7 +1401,7 @@ static int fuse_dir_fsync(struct file *file, loff_t start, loff_t end,
	struct fuse_conn *fc = get_fuse_conn(inode);
	int err;

	if (is_bad_inode(inode))
	if (fuse_is_bad(inode))
		return -EIO;

	if (fc->no_fsyncdir)
@@ -1679,7 +1700,7 @@ int fuse_do_setattr(struct dentry *dentry, struct iattr *attr,

	if (fuse_invalid_attr(&outarg.attr) ||
	    (inode->i_mode ^ outarg.attr.mode) & S_IFMT) {
		make_bad_inode(inode);
		fuse_make_bad(inode);
		err = -EIO;
		goto error;
	}
@@ -1742,6 +1763,9 @@ static int fuse_setattr(struct dentry *entry, struct iattr *attr)
	struct file *file = (attr->ia_valid & ATTR_FILE) ? attr->ia_file : NULL;
	int ret;

	if (fuse_is_bad(inode))
		return -EIO;

	if (!fuse_allow_current_process(get_fuse_conn(inode)))
		return -EACCES;

@@ -1800,6 +1824,9 @@ static int fuse_getattr(const struct path *path, struct kstat *stat,
	struct inode *inode = d_inode(path->dentry);
	struct fuse_conn *fc = get_fuse_conn(inode);

	if (fuse_is_bad(inode))
		return -EIO;

	if (!fuse_allow_current_process(fc)) {
		if (!request_mask) {
			/*
+11 −8
Original line number Diff line number Diff line
@@ -232,6 +232,9 @@ int fuse_open_common(struct inode *inode, struct file *file, bool isdir)
	bool dax_truncate = (file->f_flags & O_TRUNC) &&
			  fc->atomic_o_trunc && FUSE_IS_DAX(inode);

	if (fuse_is_bad(inode))
		return -EIO;

	err = generic_file_open(inode, file);
	if (err)
		return err;
@@ -469,7 +472,7 @@ static int fuse_flush(struct file *file, fl_owner_t id)
	FUSE_ARGS(args);
	int err;

	if (is_bad_inode(inode))
	if (fuse_is_bad(inode))
		return -EIO;

	err = write_inode_now(inode, 1);
@@ -541,7 +544,7 @@ static int fuse_fsync(struct file *file, loff_t start, loff_t end,
	struct fuse_conn *fc = get_fuse_conn(inode);
	int err;

	if (is_bad_inode(inode))
	if (fuse_is_bad(inode))
		return -EIO;

	inode_lock(inode);
@@ -865,7 +868,7 @@ static int fuse_readpage(struct file *file, struct page *page)
	int err;

	err = -EIO;
	if (is_bad_inode(inode))
	if (fuse_is_bad(inode))
		goto out;

	err = fuse_do_readpage(file, page);
@@ -958,7 +961,7 @@ static void fuse_readahead(struct readahead_control *rac)
	struct fuse_conn *fc = get_fuse_conn(inode);
	unsigned int i, max_pages, nr_pages = 0;

	if (is_bad_inode(inode))
	if (fuse_is_bad(inode))
		return;

	max_pages = min_t(unsigned int, fc->max_pages,
@@ -1570,7 +1573,7 @@ static ssize_t fuse_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
	struct fuse_file *ff = file->private_data;
	struct inode *inode = file_inode(file);

	if (is_bad_inode(inode))
	if (fuse_is_bad(inode))
		return -EIO;

	if (FUSE_IS_DAX(inode))
@@ -1588,7 +1591,7 @@ static ssize_t fuse_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
	struct fuse_file *ff = file->private_data;
	struct inode *inode = file_inode(file);

	if (is_bad_inode(inode))
	if (fuse_is_bad(inode))
		return -EIO;

	if (FUSE_IS_DAX(inode))
@@ -2187,7 +2190,7 @@ static int fuse_writepages(struct address_space *mapping,
	int err;

	err = -EIO;
	if (is_bad_inode(inode))
	if (fuse_is_bad(inode))
		goto out;

	data.inode = inode;
@@ -2972,7 +2975,7 @@ long fuse_ioctl_common(struct file *file, unsigned int cmd,
	if (!fuse_allow_current_process(fc))
		return -EACCES;

	if (is_bad_inode(inode))
	if (fuse_is_bad(inode))
		return -EIO;

	return fuse_do_ioctl(file, cmd, arg, flags);
+12 −0
Original line number Diff line number Diff line
@@ -172,6 +172,8 @@ enum {
	FUSE_I_INIT_RDPLUS,
	/** An operation changing file size is in progress  */
	FUSE_I_SIZE_UNSTABLE,
	/* Bad inode */
	FUSE_I_BAD,
};

struct fuse_conn;
@@ -859,6 +861,16 @@ static inline u64 fuse_get_attr_version(struct fuse_conn *fc)
	return atomic64_read(&fc->attr_version);
}

static inline void fuse_make_bad(struct inode *inode)
{
	set_bit(FUSE_I_BAD, &get_fuse_inode(inode)->state);
}

static inline bool fuse_is_bad(struct inode *inode)
{
	return unlikely(test_bit(FUSE_I_BAD, &get_fuse_inode(inode)->state));
}

/** Device operations */
extern const struct file_operations fuse_dev_operations;

+2 −2
Original line number Diff line number Diff line
@@ -132,7 +132,7 @@ static void fuse_evict_inode(struct inode *inode)
			fi->forget = NULL;
		}
	}
	if (S_ISREG(inode->i_mode) && !is_bad_inode(inode)) {
	if (S_ISREG(inode->i_mode) && !fuse_is_bad(inode)) {
		WARN_ON(!list_empty(&fi->write_files));
		WARN_ON(!list_empty(&fi->queued_writes));
	}
@@ -352,7 +352,7 @@ retry:
		unlock_new_inode(inode);
	} else if ((inode->i_mode ^ attr->mode) & S_IFMT) {
		/* Inode has changed type, any I/O on the old should fail */
		make_bad_inode(inode);
		fuse_make_bad(inode);
		iput(inode);
		goto retry;
	}
Loading