Pull overlayfs fixes from Miklos Szeredi: "Fix a regression in 4.14 and one in 4.13. The latter is a case when Docker is doing something it really shouldn't and gets away with it. We now print a warning instead of erroring out. There are also fixes to several error paths" * 'overlayfs-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/vfs: ovl: fix regression caused by exclusive upper/work dir protection ovl: fix missing unlock_rename() in ovl_do_copy_up() ovl: fix dentry leak in ovl_indexdir_cleanup() ovl: fix dput() of ERR_PTR in ovl_cleanup_index() ovl: fix error value printed in ovl_lookup_index() ovl: fix may_write_real() for overlayfs directories
Unified · Split
| @@ -210,8 +210,11 @@ path as another overlay mount and it may use a lower layer path that is · 209 unmodified lines | ||
| 210 | 210 | beneath or above the path of another overlay lower layer path. |
| 211 | 211 | |
| 212 | 212 | Using an upper layer path and/or a workdir path that are already used by |
| 213 | −another overlay mount is not allowed and will fail with EBUSY. Using | |
| 213 | +another overlay mount is not allowed and may fail with EBUSY. Using | |
| 214 | 214 | partially overlapping paths is not allowed but will not fail with EBUSY. |
| 215 | +If files are accessed from two overlayfs mounts which share or overlap the | |
| 216 | +upper layer and/or workdir path the behavior of the overlay is undefined, | |
| 217 | +though it will not result in a crash or deadlock. | |
| 215 | 218 | |
| 216 | 219 | Mounting an overlay using an upper layer path, where the upper layer path |
| 217 | 220 | was previously used by another mounted overlay in combination with a |
| @@ -468,7 +468,9 @@ static inline int may_write_real(struct file *file) · 467 unmodified lines | ||
| 468 | 468 | |
| 469 | 469 | /* File refers to upper, writable layer? */ |
| 470 | 470 | upperdentry = d_real(dentry, NULL, 0, D_REAL_UPPER); |
| 471 | − if (upperdentry && file_inode(file) == d_inode(upperdentry)) | |
| 471 | + if (upperdentry && | |
| 472 | + (file_inode(file) == d_inode(upperdentry) || | |
| 473 | + file_inode(file) == d_inode(dentry))) | |
| 472 | 474 | return 0; |
| 473 | 475 | |
| 474 | 476 | /* Lower layer: can't write to real file, sorry... */ |
| @@ -561,10 +561,8 @@ static int ovl_do_copy_up(struct ovl_copy_up_ctx *c) · 560 unmodified lines | ||
| 561 | 561 | c->tmpfile = true; |
| 562 | 562 | err = ovl_copy_up_locked(c); |
| 563 | 563 | } else { |
| 564 | − err = -EIO; | |
| 565 | − if (lock_rename(c->workdir, c->destdir) != NULL) { | |
| 566 | − pr_err("overlayfs: failed to lock workdir+upperdir\n"); | |
| 567 | − } else { | |
| 564 | + err = ovl_lock_rename_workdir(c->workdir, c->destdir); | |
| 565 | + if (!err) { | |
| 568 | 566 | err = ovl_copy_up_locked(c); |
| 569 | 567 | unlock_rename(c->workdir, c->destdir); |
| 570 | 568 | } |
| @@ -216,26 +216,6 @@ out_unlock: · 215 unmodified lines | ||
| 216 | 216 | return err; |
| 217 | 217 | } |
| 218 | 218 | |
| 219 | −static int ovl_lock_rename_workdir(struct dentry *workdir, | |
| 220 | − struct dentry *upperdir) | |
| 221 | −{ | |
| 222 | − /* Workdir should not be the same as upperdir */ | |
| 223 | − if (workdir == upperdir) | |
| 224 | − goto err; | |
| 225 | − | |
| 226 | − /* Workdir should not be subdir of upperdir and vice versa */ | |
| 227 | − if (lock_rename(workdir, upperdir) != NULL) | |
| 228 | − goto err_unlock; | |
| 229 | − | |
| 230 | − return 0; | |
| 231 | − | |
| 232 | −err_unlock: | |
| 233 | − unlock_rename(workdir, upperdir); | |
| 234 | −err: | |
| 235 | − pr_err("overlayfs: failed to lock workdir+upperdir\n"); | |
| 236 | − return -EIO; | |
| 237 | −} | |
| 238 | − | |
| 239 | 219 | static struct dentry *ovl_clear_empty(struct dentry *dentry, |
| 240 | 220 | struct list_head *list) |
| 241 | 221 | { |
| @@ -506,6 +506,7 @@ static struct dentry *ovl_lookup_index(struct dentry *dentry, · 505 unmodified lines | ||
| 506 | 506 | |
| 507 | 507 | index = lookup_one_len_unlocked(name.name, ofs->indexdir, name.len); |
| 508 | 508 | if (IS_ERR(index)) { |
| 509 | + err = PTR_ERR(index); | |
| 509 | 510 | pr_warn_ratelimited("overlayfs: failed inode index lookup (ino=%lu, key=%*s, err=%i);\n" |
| 510 | 511 | "overlayfs: mount with '-o index=off' to disable inodes index.\n", |
| 511 | 512 | d_inode(origin)->i_ino, name.len, name.name, |
| @@ -235,6 +235,7 @@ bool ovl_inuse_trylock(struct dentry *dentry); · 234 unmodified lines | ||
| 235 | 235 | void ovl_inuse_unlock(struct dentry *dentry); |
| 236 | 236 | int ovl_nlink_start(struct dentry *dentry, bool *locked); |
| 237 | 237 | void ovl_nlink_end(struct dentry *dentry, bool locked); |
| 238 | +int ovl_lock_rename_workdir(struct dentry *workdir, struct dentry *upperdir); | |
| 238 | 239 | |
| 239 | 240 | static inline bool ovl_is_impuredir(struct dentry *dentry) |
| 240 | 241 | { |
| @@ -37,6 +37,9 @@ struct ovl_fs { · 36 unmodified lines | ||
| 37 | 37 | bool noxattr; |
| 38 | 38 | /* sb common to all layers */ |
| 39 | 39 | struct super_block *same_sb; |
| 40 | + /* Did we take the inuse lock? */ | |
| 41 | + bool upperdir_locked; | |
| 42 | + bool workdir_locked; | |
| 40 | 43 | }; |
| 41 | 44 | |
| 42 | 45 | /* private information held for every overlayfs dentry */ |
| @@ -988,6 +988,7 @@ int ovl_indexdir_cleanup(struct dentry *dentry, struct vfsmount *mnt, · 987 unmodified lines | ||
| 988 | 988 | struct path *lowerstack, unsigned int numlower) |
| 989 | 989 | { |
| 990 | 990 | int err; |
| 991 | + struct dentry *index = NULL; | |
| 991 | 992 | struct inode *dir = dentry->d_inode; |
| 992 | 993 | struct path path = { .mnt = mnt, .dentry = dentry }; |
| 993 | 994 | LIST_HEAD(list); |
| @@ -1007,8 +1008,6 @@ int ovl_indexdir_cleanup(struct dentry *dentry, struct vfsmount *mnt, · 13 unmodified lines | ||
| 1007 | 1008 | |
| 1008 | 1009 | inode_lock_nested(dir, I_MUTEX_PARENT); |
| 1009 | 1010 | list_for_each_entry(p, &list, l_node) { |
| 1010 | − struct dentry *index; | |
| 1011 | − | |
| 1012 | 1011 | if (p->name[0] == '.') { |
| 1013 | 1012 | if (p->len == 1) |
| 1014 | 1013 | continue; |
| @@ -1018,6 +1017,7 @@ int ovl_indexdir_cleanup(struct dentry *dentry, struct vfsmount *mnt, · 3 unmodified lines | ||
| 1018 | 1017 | index = lookup_one_len(p->name, dentry, p->len); |
| 1019 | 1018 | if (IS_ERR(index)) { |
| 1020 | 1019 | err = PTR_ERR(index); |
| 1020 | + index = NULL; | |
| 1021 | 1021 | break; |
| 1022 | 1022 | } |
| 1023 | 1023 | err = ovl_verify_index(index, lowerstack, numlower); |
| @@ -1029,7 +1029,9 @@ int ovl_indexdir_cleanup(struct dentry *dentry, struct vfsmount *mnt, · 5 unmodified lines | ||
| 1029 | 1029 | break; |
| 1030 | 1030 | } |
| 1031 | 1031 | dput(index); |
| 1032 | + index = NULL; | |
| 1032 | 1033 | } |
| 1034 | + dput(index); | |
| 1033 | 1035 | inode_unlock(dir); |
| 1034 | 1036 | out: |
| 1035 | 1037 | ovl_cache_free(&list); |
| @@ -211,9 +211,10 @@ static void ovl_put_super(struct super_block *sb) · 210 unmodified lines | ||
| 211 | 211 | |
| 212 | 212 | dput(ufs->indexdir); |
| 213 | 213 | dput(ufs->workdir); |
| 214 | − ovl_inuse_unlock(ufs->workbasedir); | |
| 214 | + if (ufs->workdir_locked) | |
| 215 | + ovl_inuse_unlock(ufs->workbasedir); | |
| 215 | 216 | dput(ufs->workbasedir); |
| 216 | − if (ufs->upper_mnt) | |
| 217 | + if (ufs->upper_mnt && ufs->upperdir_locked) | |
| 217 | 218 | ovl_inuse_unlock(ufs->upper_mnt->mnt_root); |
| 218 | 219 | mntput(ufs->upper_mnt); |
| 219 | 220 | for (i = 0; i < ufs->numlower; i++) |
| @@ -881,9 +882,13 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent) · 661 unmodified lines | ||
| 881 | 882 | goto out_put_upperpath; |
| 882 | 883 | |
| 883 | 884 | err = -EBUSY; |
| 884 | − if (!ovl_inuse_trylock(upperpath.dentry)) { | |
| 885 | − pr_err("overlayfs: upperdir is in-use by another mount\n"); | |
| 885 | + if (ovl_inuse_trylock(upperpath.dentry)) { | |
| 886 | + ufs->upperdir_locked = true; | |
| 887 | + } else if (ufs->config.index) { | |
| 888 | + pr_err("overlayfs: upperdir is in-use by another mount, mount with '-o index=off' to override exclusive upperdir protection.\n"); | |
| 886 | 889 | goto out_put_upperpath; |
| 890 | + } else { | |
| 891 | + pr_warn("overlayfs: upperdir is in-use by another mount, accessing files from both mounts will result in undefined behavior.\n"); | |
| 887 | 892 | } |
| 888 | 893 | |
| 889 | 894 | err = ovl_mount_dir(ufs->config.workdir, &workpath); |
| @@ -901,9 +906,13 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent) · 11 unmodified lines | ||
| 901 | 906 | } |
| 902 | 907 | |
| 903 | 908 | err = -EBUSY; |
| 904 | − if (!ovl_inuse_trylock(workpath.dentry)) { | |
| 905 | − pr_err("overlayfs: workdir is in-use by another mount\n"); | |
| 909 | + if (ovl_inuse_trylock(workpath.dentry)) { | |
| 910 | + ufs->workdir_locked = true; | |
| 911 | + } else if (ufs->config.index) { | |
| 912 | + pr_err("overlayfs: workdir is in-use by another mount, mount with '-o index=off' to override exclusive workdir protection.\n"); | |
| 906 | 913 | goto out_put_workpath; |
| 914 | + } else { | |
| 915 | + pr_warn("overlayfs: workdir is in-use by another mount, accessing files from both mounts will result in undefined behavior.\n"); | |
| 907 | 916 | } |
| 908 | 917 | |
| 909 | 918 | ufs->workbasedir = workpath.dentry; |
| @@ -1156,11 +1165,13 @@ out_put_lowerpath: · 246 unmodified lines | ||
| 1156 | 1165 | out_free_lowertmp: |
| 1157 | 1166 | kfree(lowertmp); |
| 1158 | 1167 | out_unlock_workdentry: |
| 1159 | − ovl_inuse_unlock(workpath.dentry); | |
| 1168 | + if (ufs->workdir_locked) | |
| 1169 | + ovl_inuse_unlock(workpath.dentry); | |
| 1160 | 1170 | out_put_workpath: |
| 1161 | 1171 | path_put(&workpath); |
| 1162 | 1172 | out_unlock_upperdentry: |
| 1163 | − ovl_inuse_unlock(upperpath.dentry); | |
| 1173 | + if (ufs->upperdir_locked) | |
| 1174 | + ovl_inuse_unlock(upperpath.dentry); | |
| 1164 | 1175 | out_put_upperpath: |
| 1165 | 1176 | path_put(&upperpath); |
| 1166 | 1177 | out_free_config: |
| @@ -430,7 +430,7 @@ void ovl_inuse_unlock(struct dentry *dentry) · 429 unmodified lines | ||
| 430 | 430 | } |
| 431 | 431 | } |
| 432 | 432 | |
| 433 | −/* Called must hold OVL_I(inode)->oi_lock */ | |
| 433 | +/* Caller must hold OVL_I(inode)->lock */ | |
| 434 | 434 | static void ovl_cleanup_index(struct dentry *dentry) |
| 435 | 435 | { |
| 436 | 436 | struct inode *dir = ovl_indexdir(dentry->d_sb)->d_inode; |
| @@ -469,6 +469,9 @@ static void ovl_cleanup_index(struct dentry *dentry) · 32 unmodified lines | ||
| 469 | 469 | err = PTR_ERR(index); |
| 470 | 470 | if (!IS_ERR(index)) |
| 471 | 471 | err = ovl_cleanup(dir, index); |
| 472 | + else | |
| 473 | + index = NULL; | |
| 474 | + | |
| 472 | 475 | inode_unlock(dir); |
| 473 | 476 | if (err) |
| 474 | 477 | goto fail; |
| @@ -557,3 +560,22 @@ void ovl_nlink_end(struct dentry *dentry, bool locked) · 82 unmodified lines | ||
| 557 | 560 | mutex_unlock(&OVL_I(d_inode(dentry))->lock); |
| 558 | 561 | } |
| 559 | 562 | } |
| 563 | + | |
| 564 | +int ovl_lock_rename_workdir(struct dentry *workdir, struct dentry *upperdir) | |
| 565 | +{ | |
| 566 | + /* Workdir should not be the same as upperdir */ | |
| 567 | + if (workdir == upperdir) | |
| 568 | + goto err; | |
| 569 | + | |
| 570 | + /* Workdir should not be subdir of upperdir and vice versa */ | |
| 571 | + if (lock_rename(workdir, upperdir) != NULL) | |
| 572 | + goto err_unlock; | |
| 573 | + | |
| 574 | + return 0; | |
| 575 | + | |
| 576 | +err_unlock: | |
| 577 | + unlock_rename(workdir, upperdir); | |
| 578 | +err: | |
| 579 | + pr_err("overlayfs: failed to lock workdir+upperdir\n"); | |
| 580 | + return -EIO; | |
| 581 | +} | |