Skip to content

Commit 5b02bfc

Browse files
amir73ilamir73il
committed
ovl: do not encode lower fh with upper sb_writers held
When lower fs is a nested overlayfs, calling encode_fh() on a lower directory dentry may trigger copy up and take sb_writers on the upper fs of the lower nested overlayfs. The lower nested overlayfs may have the same upper fs as this overlayfs, so nested sb_writers lock is illegal. Move all the callers that encode lower fh to before ovl_want_write(). Signed-off-by: Amir Goldstein <[email protected]>
1 parent c63e56a commit 5b02bfc

File tree

5 files changed

+104
-43
lines changed

5 files changed

+104
-43
lines changed

‎fs/overlayfs/copy_up.c

Lines changed: 33 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -434,29 +434,29 @@ struct ovl_fh *ovl_encode_real_fh(struct ovl_fs *ofs, struct dentry *real,
434434
return ERR_PTR(err);
435435
}
436436

437-
int ovl_set_origin(struct ovl_fs *ofs, struct dentry *lower,
438-
struct dentry *upper)
437+
struct ovl_fh *ovl_get_origin_fh(struct ovl_fs *ofs, struct dentry *origin)
439438
{
440-
const struct ovl_fh *fh = NULL;
441-
int err;
442-
443439
/*
444440
* When lower layer doesn't support export operations store a 'null' fh,
445441
* so we can use the overlay.origin xattr to distignuish between a copy
446442
* up and a pure upper inode.
447443
*/
448-
if (ovl_can_decode_fh(lower->d_sb)) {
449-
fh = ovl_encode_real_fh(ofs, lower, false);
450-
if (IS_ERR(fh))
451-
return PTR_ERR(fh);
452-
}
444+
if (!ovl_can_decode_fh(origin->d_sb))
445+
return NULL;
446+
447+
return ovl_encode_real_fh(ofs, origin, false);
448+
}
449+
450+
int ovl_set_origin_fh(struct ovl_fs *ofs, const struct ovl_fh *fh,
451+
struct dentry *upper)
452+
{
453+
int err;
453454

454455
/*
455456
* Do not fail when upper doesn't support xattrs.
456457
*/
457458
err = ovl_check_setxattr(ofs, upper, OVL_XATTR_ORIGIN, fh->buf,
458459
fh ? fh->fb.len : 0, 0);
459-
kfree(fh);
460460

461461
/* Ignore -EPERM from setting "user.*" on symlink/special */
462462
return err == -EPERM ? 0 : err;
@@ -484,7 +484,7 @@ static int ovl_set_upper_fh(struct ovl_fs *ofs, struct dentry *upper,
484484
*
485485
* Caller must hold i_mutex on indexdir.
486486
*/
487-
static int ovl_create_index(struct dentry *dentry, struct dentry *origin,
487+
static int ovl_create_index(struct dentry *dentry, const struct ovl_fh *fh,
488488
struct dentry *upper)
489489
{
490490
struct ovl_fs *ofs = OVL_FS(dentry->d_sb);
@@ -510,7 +510,7 @@ static int ovl_create_index(struct dentry *dentry, struct dentry *origin,
510510
if (WARN_ON(ovl_test_flag(OVL_INDEX, d_inode(dentry))))
511511
return -EIO;
512512

513-
err = ovl_get_index_name(ofs, origin, &name);
513+
err = ovl_get_index_name_fh(fh, &name);
514514
if (err)
515515
return err;
516516

@@ -549,6 +549,7 @@ struct ovl_copy_up_ctx {
549549
struct dentry *destdir;
550550
struct qstr destname;
551551
struct dentry *workdir;
552+
const struct ovl_fh *origin_fh;
552553
bool origin;
553554
bool indexed;
554555
bool metacopy;
@@ -649,7 +650,7 @@ static int ovl_copy_up_metadata(struct ovl_copy_up_ctx *c, struct dentry *temp)
649650
* hard link.
650651
*/
651652
if (c->origin) {
652-
err = ovl_set_origin(ofs, c->lowerpath.dentry, temp);
653+
err = ovl_set_origin_fh(ofs, c->origin_fh, temp);
653654
if (err)
654655
return err;
655656
}
@@ -772,7 +773,7 @@ static int ovl_copy_up_workdir(struct ovl_copy_up_ctx *c)
772773
goto cleanup;
773774

774775
if (S_ISDIR(c->stat.mode) && c->indexed) {
775-
err = ovl_create_index(c->dentry, c->lowerpath.dentry, temp);
776+
err = ovl_create_index(c->dentry, c->origin_fh, temp);
776777
if (err)
777778
goto cleanup;
778779
}
@@ -890,6 +891,8 @@ static int ovl_do_copy_up(struct ovl_copy_up_ctx *c)
890891
{
891892
int err;
892893
struct ovl_fs *ofs = OVL_FS(c->dentry->d_sb);
894+
struct dentry *origin = c->lowerpath.dentry;
895+
struct ovl_fh *fh = NULL;
893896
bool to_index = false;
894897

895898
/*
@@ -906,17 +909,25 @@ static int ovl_do_copy_up(struct ovl_copy_up_ctx *c)
906909
to_index = true;
907910
}
908911

909-
if (S_ISDIR(c->stat.mode) || c->stat.nlink == 1 || to_index)
912+
if (S_ISDIR(c->stat.mode) || c->stat.nlink == 1 || to_index) {
913+
fh = ovl_get_origin_fh(ofs, origin);
914+
if (IS_ERR(fh))
915+
return PTR_ERR(fh);
916+
917+
/* origin_fh may be NULL */
918+
c->origin_fh = fh;
910919
c->origin = true;
920+
}
911921

912922
if (to_index) {
913923
c->destdir = ovl_indexdir(c->dentry->d_sb);
914-
err = ovl_get_index_name(ofs, c->lowerpath.dentry, &c->destname);
924+
err = ovl_get_index_name(ofs, origin, &c->destname);
915925
if (err)
916-
return err;
926+
goto out_free_fh;
917927
} else if (WARN_ON(!c->parent)) {
918928
/* Disconnected dentry must be copied up to index dir */
919-
return -EIO;
929+
err = -EIO;
930+
goto out_free_fh;
920931
} else {
921932
/*
922933
* Mark parent "impure" because it may now contain non-pure
@@ -926,7 +937,7 @@ static int ovl_do_copy_up(struct ovl_copy_up_ctx *c)
926937
err = ovl_set_impure(c->parent, c->destdir);
927938
ovl_end_write(c->dentry);
928939
if (err)
929-
return err;
940+
goto out_free_fh;
930941
}
931942

932943
/* Should we copyup with O_TMPFILE or with workdir? */
@@ -960,6 +971,8 @@ static int ovl_do_copy_up(struct ovl_copy_up_ctx *c)
960971
out:
961972
if (to_index)
962973
kfree(c->destname.name);
974+
out_free_fh:
975+
kfree(fh);
963976
return err;
964977
}
965978

‎fs/overlayfs/namei.c

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -507,6 +507,19 @@ static int ovl_verify_fh(struct ovl_fs *ofs, struct dentry *dentry,
507507
return err;
508508
}
509509

510+
int ovl_verify_set_fh(struct ovl_fs *ofs, struct dentry *dentry,
511+
enum ovl_xattr ox, const struct ovl_fh *fh,
512+
bool is_upper, bool set)
513+
{
514+
int err;
515+
516+
err = ovl_verify_fh(ofs, dentry, ox, fh);
517+
if (set && err == -ENODATA)
518+
err = ovl_setxattr(ofs, dentry, ox, fh->buf, fh->fb.len);
519+
520+
return err;
521+
}
522+
510523
/*
511524
* Verify that @real dentry matches the file handle stored in xattr @name.
512525
*
@@ -515,9 +528,9 @@ static int ovl_verify_fh(struct ovl_fs *ofs, struct dentry *dentry,
515528
*
516529
* Return 0 on match, -ESTALE on mismatch, -ENODATA on no xattr, < 0 on error.
517530
*/
518-
int ovl_verify_set_fh(struct ovl_fs *ofs, struct dentry *dentry,
519-
enum ovl_xattr ox, struct dentry *real, bool is_upper,
520-
bool set)
531+
int ovl_verify_origin_xattr(struct ovl_fs *ofs, struct dentry *dentry,
532+
enum ovl_xattr ox, struct dentry *real,
533+
bool is_upper, bool set)
521534
{
522535
struct inode *inode;
523536
struct ovl_fh *fh;
@@ -530,9 +543,7 @@ int ovl_verify_set_fh(struct ovl_fs *ofs, struct dentry *dentry,
530543
goto fail;
531544
}
532545

533-
err = ovl_verify_fh(ofs, dentry, ox, fh);
534-
if (set && err == -ENODATA)
535-
err = ovl_setxattr(ofs, dentry, ox, fh->buf, fh->fb.len);
546+
err = ovl_verify_set_fh(ofs, dentry, ox, fh, is_upper, set);
536547
if (err)
537548
goto fail;
538549

@@ -548,6 +559,7 @@ int ovl_verify_set_fh(struct ovl_fs *ofs, struct dentry *dentry,
548559
goto out;
549560
}
550561

562+
551563
/* Get upper dentry from index */
552564
struct dentry *ovl_index_upper(struct ovl_fs *ofs, struct dentry *index,
553565
bool connected)
@@ -684,7 +696,7 @@ int ovl_verify_index(struct ovl_fs *ofs, struct dentry *index)
684696
goto out;
685697
}
686698

687-
static int ovl_get_index_name_fh(struct ovl_fh *fh, struct qstr *name)
699+
int ovl_get_index_name_fh(const struct ovl_fh *fh, struct qstr *name)
688700
{
689701
char *n, *s;
690702

@@ -873,20 +885,27 @@ int ovl_path_next(int idx, struct dentry *dentry, struct path *path)
873885
static int ovl_fix_origin(struct ovl_fs *ofs, struct dentry *dentry,
874886
struct dentry *lower, struct dentry *upper)
875887
{
888+
const struct ovl_fh *fh;
876889
int err;
877890

878891
if (ovl_check_origin_xattr(ofs, upper))
879892
return 0;
880893

894+
fh = ovl_get_origin_fh(ofs, lower);
895+
if (IS_ERR(fh))
896+
return PTR_ERR(fh);
897+
881898
err = ovl_want_write(dentry);
882899
if (err)
883-
return err;
900+
goto out;
884901

885-
err = ovl_set_origin(ofs, lower, upper);
902+
err = ovl_set_origin_fh(ofs, fh, upper);
886903
if (!err)
887904
err = ovl_set_impure(dentry->d_parent, upper->d_parent);
888905

889906
ovl_drop_write(dentry);
907+
out:
908+
kfree(fh);
890909
return err;
891910
}
892911

‎fs/overlayfs/overlayfs.h

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -628,11 +628,15 @@ struct dentry *ovl_decode_real_fh(struct ovl_fs *ofs, struct ovl_fh *fh,
628628
int ovl_check_origin_fh(struct ovl_fs *ofs, struct ovl_fh *fh, bool connected,
629629
struct dentry *upperdentry, struct ovl_path **stackp);
630630
int ovl_verify_set_fh(struct ovl_fs *ofs, struct dentry *dentry,
631-
enum ovl_xattr ox, struct dentry *real, bool is_upper,
632-
bool set);
631+
enum ovl_xattr ox, const struct ovl_fh *fh,
632+
bool is_upper, bool set);
633+
int ovl_verify_origin_xattr(struct ovl_fs *ofs, struct dentry *dentry,
634+
enum ovl_xattr ox, struct dentry *real,
635+
bool is_upper, bool set);
633636
struct dentry *ovl_index_upper(struct ovl_fs *ofs, struct dentry *index,
634637
bool connected);
635638
int ovl_verify_index(struct ovl_fs *ofs, struct dentry *index);
639+
int ovl_get_index_name_fh(const struct ovl_fh *fh, struct qstr *name);
636640
int ovl_get_index_name(struct ovl_fs *ofs, struct dentry *origin,
637641
struct qstr *name);
638642
struct dentry *ovl_get_index_fh(struct ovl_fs *ofs, struct ovl_fh *fh);
@@ -644,17 +648,24 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
644648
unsigned int flags);
645649
bool ovl_lower_positive(struct dentry *dentry);
646650

651+
static inline int ovl_verify_origin_fh(struct ovl_fs *ofs, struct dentry *upper,
652+
const struct ovl_fh *fh, bool set)
653+
{
654+
return ovl_verify_set_fh(ofs, upper, OVL_XATTR_ORIGIN, fh, false, set);
655+
}
656+
647657
static inline int ovl_verify_origin(struct ovl_fs *ofs, struct dentry *upper,
648658
struct dentry *origin, bool set)
649659
{
650-
return ovl_verify_set_fh(ofs, upper, OVL_XATTR_ORIGIN, origin,
651-
false, set);
660+
return ovl_verify_origin_xattr(ofs, upper, OVL_XATTR_ORIGIN, origin,
661+
false, set);
652662
}
653663

654664
static inline int ovl_verify_upper(struct ovl_fs *ofs, struct dentry *index,
655665
struct dentry *upper, bool set)
656666
{
657-
return ovl_verify_set_fh(ofs, index, OVL_XATTR_UPPER, upper, true, set);
667+
return ovl_verify_origin_xattr(ofs, index, OVL_XATTR_UPPER, upper,
668+
true, set);
658669
}
659670

660671
/* readdir.c */
@@ -819,8 +830,9 @@ int ovl_copy_xattr(struct super_block *sb, const struct path *path, struct dentr
819830
int ovl_set_attr(struct ovl_fs *ofs, struct dentry *upper, struct kstat *stat);
820831
struct ovl_fh *ovl_encode_real_fh(struct ovl_fs *ofs, struct dentry *real,
821832
bool is_upper);
822-
int ovl_set_origin(struct ovl_fs *ofs, struct dentry *lower,
823-
struct dentry *upper);
833+
struct ovl_fh *ovl_get_origin_fh(struct ovl_fs *ofs, struct dentry *origin);
834+
int ovl_set_origin_fh(struct ovl_fs *ofs, const struct ovl_fh *fh,
835+
struct dentry *upper);
824836

825837
/* export.c */
826838
extern const struct export_operations ovl_export_operations;

‎fs/overlayfs/super.c

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -887,15 +887,20 @@ static int ovl_get_indexdir(struct super_block *sb, struct ovl_fs *ofs,
887887
{
888888
struct vfsmount *mnt = ovl_upper_mnt(ofs);
889889
struct dentry *indexdir;
890+
struct dentry *origin = ovl_lowerstack(oe)->dentry;
891+
const struct ovl_fh *fh;
890892
int err;
891893

894+
fh = ovl_get_origin_fh(ofs, origin);
895+
if (IS_ERR(fh))
896+
return PTR_ERR(fh);
897+
892898
err = mnt_want_write(mnt);
893899
if (err)
894-
return err;
900+
goto out_free_fh;
895901

896902
/* Verify lower root is upper root origin */
897-
err = ovl_verify_origin(ofs, upperpath->dentry,
898-
ovl_lowerstack(oe)->dentry, true);
903+
err = ovl_verify_origin_fh(ofs, upperpath->dentry, fh, true);
899904
if (err) {
900905
pr_err("failed to verify upper root origin\n");
901906
goto out;
@@ -927,9 +932,10 @@ static int ovl_get_indexdir(struct super_block *sb, struct ovl_fs *ofs,
927932
* directory entries.
928933
*/
929934
if (ovl_check_origin_xattr(ofs, ofs->indexdir)) {
930-
err = ovl_verify_set_fh(ofs, ofs->indexdir,
931-
OVL_XATTR_ORIGIN,
932-
upperpath->dentry, true, false);
935+
err = ovl_verify_origin_xattr(ofs, ofs->indexdir,
936+
OVL_XATTR_ORIGIN,
937+
upperpath->dentry, true,
938+
false);
933939
if (err)
934940
pr_err("failed to verify index dir 'origin' xattr\n");
935941
}
@@ -947,6 +953,8 @@ static int ovl_get_indexdir(struct super_block *sb, struct ovl_fs *ofs,
947953

948954
out:
949955
mnt_drop_write(mnt);
956+
out_free_fh:
957+
kfree(fh);
950958
return err;
951959
}
952960

‎fs/overlayfs/util.c

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1013,12 +1013,18 @@ static void ovl_cleanup_index(struct dentry *dentry)
10131013
struct dentry *index = NULL;
10141014
struct inode *inode;
10151015
struct qstr name = { };
1016+
bool got_write = false;
10161017
int err;
10171018

10181019
err = ovl_get_index_name(ofs, lowerdentry, &name);
10191020
if (err)
10201021
goto fail;
10211022

1023+
err = ovl_want_write(dentry);
1024+
if (err)
1025+
goto fail;
1026+
1027+
got_write = true;
10221028
inode = d_inode(upperdentry);
10231029
if (!S_ISDIR(inode->i_mode) && inode->i_nlink != 1) {
10241030
pr_warn_ratelimited("cleanup linked index (%pd2, ino=%lu, nlink=%u)\n",
@@ -1056,6 +1062,8 @@ static void ovl_cleanup_index(struct dentry *dentry)
10561062
goto fail;
10571063

10581064
out:
1065+
if (got_write)
1066+
ovl_drop_write(dentry);
10591067
kfree(name.name);
10601068
dput(index);
10611069
return;
@@ -1135,6 +1143,8 @@ void ovl_nlink_end(struct dentry *dentry)
11351143
{
11361144
struct inode *inode = d_inode(dentry);
11371145

1146+
ovl_drop_write(dentry);
1147+
11381148
if (ovl_test_flag(OVL_INDEX, inode) && inode->i_nlink == 0) {
11391149
const struct cred *old_cred;
11401150

@@ -1143,7 +1153,6 @@ void ovl_nlink_end(struct dentry *dentry)
11431153
revert_creds(old_cred);
11441154
}
11451155

1146-
ovl_drop_write(dentry);
11471156
ovl_inode_unlock(inode);
11481157
}
11491158

0 commit comments

Comments
 (0)