On Fri, Oct 04, 2024 at 12:23:39PM +0200, Amir Goldstein wrote:
> ovl_fsync() with !datasync opens a backing file from the top most dentry
> in the stack, checks if this dentry is non-upper and skips the fsync.
>
> In case of an overlay dentry stack with lower data and lower metadata
> above it, but without an upper metadata above it, the backing file is
> opened from the top most lower metadata dentry and never used.
>
> Fix the helper ovl_real_fdget_meta() to return an empty struct fd in
> that case to avoid the unneeded backing file open.
Umm... Won't that screw the callers of ovl_real_fd()?
I mean, here
> @@ -395,7 +397,7 @@ static int ovl_fsync(struct file *file, loff_t start, loff_t end, int datasync)
> return ret;
>
> ret = ovl_real_fdget_meta(file, &real, !datasync);
> - if (ret)
> + if (ret || fd_empty(real))
> return ret;
>
you are taking account of that, but what of e.g.
ret = ovl_real_fdget(file, &real);
if (ret)
return ret;
/*
* Overlay file f_pos is the master copy that is preserved
* through copy up and modified on read/write, but only real
* fs knows how to SEEK_HOLE/SEEK_DATA and real fs may impose
* limitations that are more strict than ->s_maxbytes for specific
* files, so we use the real file to perform seeks.
*/
ovl_inode_lock(inode);
fd_file(real)->f_pos = file->f_pos;
in ovl_llseek()? Get ovl_real_fdget_meta() called by ovl_real_fdget() and
have it return 0 with NULL in fd_file(real), and you've got an oops right
there, don't you?
At the very least it's a bisect hazard...
