On Sun, Oct 06, 2024 at 10:23:58AM +0200, Amir Goldstein wrote:
> Stop using struct fd to return a real file from ovl_real_fdget_path(),
> because we no longer return a temporary file object and the callers
> always get a borrowed file reference.
>
> Rename the helper to ovl_real_file_path(), return a borrowed reference
> of the real file that is referenced from the overlayfs file or an error.
>
> Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
> ---
> fs/overlayfs/file.c | 70 +++++++++++++++++++++++++--------------------
> 1 file changed, 39 insertions(+), 31 deletions(-)
>
> diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
> index 42f9bbdd65b4..ead805e9f2d6 100644
> --- a/fs/overlayfs/file.c
> +++ b/fs/overlayfs/file.c
> +static struct file *ovl_upper_file(const struct file *file, bool data)
> {
> struct dentry *dentry = file_dentry(file);
> struct path realpath;
> @@ -193,12 +204,11 @@ static int ovl_upper_fdget(const struct file *file, struct fd *real, bool data)
> else
> type = ovl_path_real(dentry, &realpath);
>
> - real->word = 0;
> /* Not interested in lower nor in upper meta if data was requested */
> if (!OVL_TYPE_UPPER(type) || (data && OVL_TYPE_MERGE(type)))
> - return 0;
> + return NULL;
>
> - return ovl_real_fdget_path(file, real, &realpath);
> + return ovl_real_file_path(file, &realpath);
AFAICS, we should never get NULL from ovl_real_file_path() now.
> static int ovl_open(struct inode *inode, struct file *file)
> @@ -455,7 +465,7 @@ static ssize_t ovl_splice_write(struct pipe_inode_info *pipe, struct file *out,
>
> static int ovl_fsync(struct file *file, loff_t start, loff_t end, int datasync)
> {
> - struct fd real;
> + struct file *realfile;
> const struct cred *old_cred;
> int ret;
>
> @@ -463,19 +473,17 @@ static int ovl_fsync(struct file *file, loff_t start, loff_t end, int datasync)
> if (ret <= 0)
> return ret;
>
> - ret = ovl_upper_fdget(file, &real, datasync);
> - if (ret || fd_empty(real))
> - return ret;
> + realfile = ovl_upper_file(file, datasync);
> + if (IS_ERR_OR_NULL(realfile))
> + return PTR_ERR(realfile);
... if so, the only source of NULL here would be the checks for OVL_TYPE_...
in ovl_upper_file(). Which has no other callers...
> /* Don't sync lower file for fear of receiving EROFS error */
> - if (file_inode(fd_file(real)) == ovl_inode_upper(file_inode(file))) {
> + if (file_inode(realfile) == ovl_inode_upper(file_inode(file))) {
Can that _not_ be true after the same checks in ovl_upper_file()?
