ovl: change ovl_create_real() to get a new lock when re-opening created file.

When ovl_create_real() is used to create a file on the upper filesystem
it needs to return the resulting dentry - positive and hashed.
It is usually the case the that dentry passed to the create function
(e.g. vfs_create()) will be suitable but this is not guaranteed. The
filesystem may unhash that dentry forcing a repeat lookup next time the
name is wanted.

So ovl_create_real() must be (and is) aware of this and prepared to
perform that lookup to get a hash positive dentry.

This is currently done under that same directory lock that provided
exclusion for the create. Proposed changes to locking will make this
not possible - as the name, rather than the directory, will be locked.
The new APIs provided for lookup and locking do not and cannot support
this pattern.

The lock isn't needed. ovl_create_real() can drop the lock and then get
a new lock for the lookup - then check that the lookup returned the
correct inode. In a well-behaved configuration where the upper
filesystem is not being modified by a third party, this will always work
reliably, and if there are separate modification it will fail cleanly.

So change ovl_create_real() to drop the lock and call
ovl_start_creating_upper() to find the correct dentry. Note that
start_creating doesn't fail if the name already exists.

The lookup previously used the name from newdentry which was guaranteed
to be stable because the parent directory was locked. As we now drop
the lock we lose that guarantee. As newdentry is unhashed it is
unlikely for the name to change, but safest not to depend on that. So
the expected name is now passed in to ovl_create_real() and that is
used.

This removes the only remaining use of ovl_lookup_upper, so it is
removed.

Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: NeilBrown <neil@brown.name>
Link: https://patch.msgid.link/20260224222542.3458677-13-neilb@ownmail.net
Signed-off-by: Christian Brauner <brauner@kernel.org>

authored by

NeilBrown and committed by

Christian Brauner 3 months ago 83482786 85bb1420

+27 -19

3 changed files

expand all

overlayfs

dir.c

overlayfs.h

super.c

+25 -12

fs/overlayfs/dir.c

··· 159 159 } 160 160 161 161 struct dentry *ovl_create_real(struct ovl_fs *ofs, struct dentry *parent, 162 - struct dentry *newdentry, struct ovl_cattr *attr) 162 + struct dentry *newdentry, struct qstr *qname, 163 + struct ovl_cattr *attr) 163 164 { 164 165 struct inode *dir = parent->d_inode; 165 166 int err; ··· 222 221 struct dentry *d; 223 222 /* 224 223 * Some filesystems (i.e. casefolded) may return an unhashed 225 - * negative dentry from the ovl_lookup_upper() call before 224 + * negative dentry from the ovl_start_creating_upper() call before 226 225 * ovl_create_real(). 227 226 * In that case, lookup again after making the newdentry 228 227 * positive, so ovl_create_upper() always returns a hashed 229 - * positive dentry. 228 + * positive dentry. We lookup using qname which should be 229 + * the same name as newentry, but is certain not to change. 230 + * As we have to drop the lock before the lookup a race 231 + * could result in a lookup failure. In that case we return 232 + * an error. 230 233 */ 231 - d = ovl_lookup_upper(ofs, newdentry->d_name.name, parent, 232 - newdentry->d_name.len); 233 - dput(newdentry); 234 - if (IS_ERR_OR_NULL(d)) 234 + end_creating_keep(newdentry); 235 + d = ovl_start_creating_upper(ofs, parent, qname); 236 + 237 + if (IS_ERR_OR_NULL(d)) { 235 238 err = d ? PTR_ERR(d) : -ENOENT; 236 - else 239 + } else if (d->d_inode != newdentry->d_inode) { 240 + err = -EIO; 241 + } else { 242 + dput(newdentry); 237 243 return d; 244 + } 245 + end_creating(d); 246 + dput(newdentry); 247 + return ERR_PTR(err); 238 248 } 239 249 out: 240 250 if (err) { ··· 264 252 ret = ovl_start_creating_temp(ofs, workdir, name); 265 253 if (IS_ERR(ret)) 266 254 return ret; 267 - ret = ovl_create_real(ofs, workdir, ret, attr); 255 + ret = ovl_create_real(ofs, workdir, ret, &QSTR(name), attr); 268 256 return end_creating_keep(ret); 269 257 } 270 258 ··· 364 352 struct ovl_fs *ofs = OVL_FS(dentry->d_sb); 365 353 struct dentry *upperdir = ovl_dentry_upper(dentry->d_parent); 366 354 struct dentry *newdentry; 355 + struct qstr qname = QSTR_LEN(dentry->d_name.name, 356 + dentry->d_name.len); 367 357 int err; 368 358 369 359 newdentry = ovl_start_creating_upper(ofs, upperdir, 370 - &QSTR_LEN(dentry->d_name.name, 371 - dentry->d_name.len)); 360 + &qname); 372 361 if (IS_ERR(newdentry)) 373 362 return PTR_ERR(newdentry); 374 - newdentry = ovl_create_real(ofs, upperdir, newdentry, attr); 363 + newdentry = ovl_create_real(ofs, upperdir, newdentry, &qname, attr); 375 364 if (IS_ERR(newdentry)) 376 365 return PTR_ERR(newdentry); 377 366

+1 -7

fs/overlayfs/overlayfs.h

··· 406 406 return file; 407 407 } 408 408 409 - static inline struct dentry *ovl_lookup_upper(struct ovl_fs *ofs, 410 - const char *name, 411 - struct dentry *base, int len) 412 - { 413 - return lookup_one(ovl_upper_mnt_idmap(ofs), &QSTR_LEN(name, len), base); 414 - } 415 - 416 409 static inline struct dentry *ovl_lookup_upper_unlocked(struct ovl_fs *ofs, 417 410 const char *name, 418 411 struct dentry *base, ··· 881 888 882 889 struct dentry *ovl_create_real(struct ovl_fs *ofs, 883 890 struct dentry *parent, struct dentry *newdentry, 891 + struct qstr *qname, 884 892 struct ovl_cattr *attr); 885 893 int ovl_cleanup(struct ovl_fs *ofs, struct dentry *workdir, struct dentry *dentry); 886 894 #define OVL_TEMPNAME_SIZE 20

fs/overlayfs/super.c

··· 634 634 if (!IS_ERR(child)) { 635 635 if (!child->d_inode) 636 636 child = ovl_create_real(ofs, parent, child, 637 + &QSTR(name), 637 638 OVL_CATTR(mode)); 638 639 end_creating_keep(child); 639 640 }

Configure Feed

Configure Feed