Unverified Commit 8afa0a73 authored by NeilBrown's avatar NeilBrown Committed by Christian Brauner
Browse files

ovl: narrow locking in ovl_whiteout() 



ovl_whiteout() relies on the workdir i_rwsem to provide exclusive access
to ofs->whiteout which it manipulates.  Rather than depending on this,
add a new mutex, "whiteout_lock" to explicitly provide the required
locking.  Use guard(mutex) for this so that we can return without
needing to explicitly unlock.

Then take the lock on workdir only when needed - to lookup the temp name
and to do the whiteout or link.

Signed-off-by: default avatarNeilBrown <neil@brown.name>
Link: https://lore.kernel.org/20250716004725.1206467-19-neil@brown.name


Reviewed-by: default avatarAmir Goldstein <amir73il@gmail.com>
Signed-off-by: default avatarChristian Brauner <brauner@kernel.org>
parent 2fa14cf2

fs/overlayfs/dir.c

+24 −20
Original line number Diff line number Diff line
@@ -84,41 +84,45 @@ static struct dentry *ovl_whiteout(struct ovl_fs *ofs) 
	struct dentry *workdir = ofs->workdir;

	struct inode *wdir = workdir->d_inode;



	inode_lock_nested(wdir, I_MUTEX_PARENT);

	guard(mutex)(&ofs->whiteout_lock);



	if (!ofs->whiteout) {

		inode_lock_nested(wdir, I_MUTEX_PARENT);

		whiteout = ovl_lookup_temp(ofs, workdir);

		if (IS_ERR(whiteout))

			goto out;



		if (!IS_ERR(whiteout)) {

			err = ovl_do_whiteout(ofs, wdir, whiteout);

			if (err) {

				dput(whiteout);

				whiteout = ERR_PTR(err);

			goto out;

			}

		}

		inode_unlock(wdir);

		if (IS_ERR(whiteout))

			return whiteout;

		ofs->whiteout = whiteout;

	}



	if (!ofs->no_shared_whiteout) {

		inode_lock_nested(wdir, I_MUTEX_PARENT);

		whiteout = ovl_lookup_temp(ofs, workdir);

		if (IS_ERR(whiteout))

			goto out;



		if (!IS_ERR(whiteout)) {

			err = ovl_do_link(ofs, ofs->whiteout, wdir, whiteout);

		if (!err)

			goto out;



		if (err != -EMLINK) {

			if (err) {

				dput(whiteout);

				whiteout = ERR_PTR(err);

			}

		}

		inode_unlock(wdir);

		if (!IS_ERR(whiteout))

			return whiteout;

		if (PTR_ERR(whiteout) != -EMLINK) {

			pr_warn("Failed to link whiteout - disabling whiteout inode sharing(nlink=%u, err=%i)\n",

				ofs->whiteout->d_inode->i_nlink, err);

			ofs->no_shared_whiteout = true;

		}

		dput(whiteout);

	}

	whiteout = ofs->whiteout;

	ofs->whiteout = NULL;

out:

	inode_unlock(wdir);

	return whiteout;

}

fs/overlayfs/ovl_entry.h

+1 −0
Original line number Diff line number Diff line
@@ -88,6 +88,7 @@ struct ovl_fs { 
	/* Shared whiteout cache */

	struct dentry *whiteout;

	bool no_shared_whiteout;

	struct mutex whiteout_lock;

	/* r/o snapshot of upperdir sb's only taken on volatile mounts */

	errseq_t errseq;

};

fs/overlayfs/params.c

+2 −0
Original line number Diff line number Diff line
@@ -795,6 +795,8 @@ int ovl_init_fs_context(struct fs_context *fc) 
	fc->s_fs_info		= ofs;

	fc->fs_private		= ctx;

	fc->ops			= &ovl_context_ops;



	mutex_init(&ofs->whiteout_lock);

	return 0;



out_err: