Skip to content

Commit 26423e1

Browse files
amir73ilgregkh
amir73il
authored and
gregkh
committed
ovl: do not encode lower fh with upper sb_writers held
[ Upstream commit 5b02bfc ] 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]> Stable-dep-of: c45beeb ("ovl: support encoding fid from inode with no alias") Signed-off-by: Sasha Levin <[email protected]>
1 parent 1e92afe commit 26423e1

File tree

5 files changed

+104
-42
lines changed

5 files changed

+104
-42
lines changed

‎fs/overlayfs/copy_up.c

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

429-
int ovl_set_origin(struct ovl_fs *ofs, struct dentry *lower,
430-
struct dentry *upper)
429+
struct ovl_fh *ovl_get_origin_fh(struct ovl_fs *ofs, struct dentry *origin)
431430
{
432-
const struct ovl_fh *fh = NULL;
433-
int err;
434-
435431
/*
436432
* When lower layer doesn't support export operations store a 'null' fh,
437433
* so we can use the overlay.origin xattr to distignuish between a copy
438434
* up and a pure upper inode.
439435
*/
440-
if (ovl_can_decode_fh(lower->d_sb)) {
441-
fh = ovl_encode_real_fh(ofs, lower, false);
442-
if (IS_ERR(fh))
443-
return PTR_ERR(fh);
444-
}
436+
if (!ovl_can_decode_fh(origin->d_sb))
437+
return NULL;
438+
439+
return ovl_encode_real_fh(ofs, origin, false);
440+
}
441+
442+
int ovl_set_origin_fh(struct ovl_fs *ofs, const struct ovl_fh *fh,
443+
struct dentry *upper)
444+
{
445+
int err;
445446

446447
/*
447448
* Do not fail when upper doesn't support xattrs.
448449
*/
449450
err = ovl_check_setxattr(ofs, upper, OVL_XATTR_ORIGIN, fh->buf,
450451
fh ? fh->fb.len : 0, 0);
451-
kfree(fh);
452452

453453
/* Ignore -EPERM from setting "user.*" on symlink/special */
454454
return err == -EPERM ? 0 : err;
@@ -476,7 +476,7 @@ static int ovl_set_upper_fh(struct ovl_fs *ofs, struct dentry *upper,
476476
*
477477
* Caller must hold i_mutex on indexdir.
478478
*/
479-
static int ovl_create_index(struct dentry *dentry, struct dentry *origin,
479+
static int ovl_create_index(struct dentry *dentry, const struct ovl_fh *fh,
480480
struct dentry *upper)
481481
{
482482
struct ovl_fs *ofs = OVL_FS(dentry->d_sb);
@@ -502,7 +502,7 @@ static int ovl_create_index(struct dentry *dentry, struct dentry *origin,
502502
if (WARN_ON(ovl_test_flag(OVL_INDEX, d_inode(dentry))))
503503
return -EIO;
504504

505-
err = ovl_get_index_name(ofs, origin, &name);
505+
err = ovl_get_index_name_fh(fh, &name);
506506
if (err)
507507
return err;
508508

@@ -541,6 +541,7 @@ struct ovl_copy_up_ctx {
541541
struct dentry *destdir;
542542
struct qstr destname;
543543
struct dentry *workdir;
544+
const struct ovl_fh *origin_fh;
544545
bool origin;
545546
bool indexed;
546547
bool metacopy;
@@ -637,7 +638,7 @@ static int ovl_copy_up_metadata(struct ovl_copy_up_ctx *c, struct dentry *temp)
637638
* hard link.
638639
*/
639640
if (c->origin) {
640-
err = ovl_set_origin(ofs, c->lowerpath.dentry, temp);
641+
err = ovl_set_origin_fh(ofs, c->origin_fh, temp);
641642
if (err)
642643
return err;
643644
}
@@ -749,7 +750,7 @@ static int ovl_copy_up_workdir(struct ovl_copy_up_ctx *c)
749750
goto cleanup;
750751

751752
if (S_ISDIR(c->stat.mode) && c->indexed) {
752-
err = ovl_create_index(c->dentry, c->lowerpath.dentry, temp);
753+
err = ovl_create_index(c->dentry, c->origin_fh, temp);
753754
if (err)
754755
goto cleanup;
755756
}
@@ -861,6 +862,8 @@ static int ovl_do_copy_up(struct ovl_copy_up_ctx *c)
861862
{
862863
int err;
863864
struct ovl_fs *ofs = OVL_FS(c->dentry->d_sb);
865+
struct dentry *origin = c->lowerpath.dentry;
866+
struct ovl_fh *fh = NULL;
864867
bool to_index = false;
865868

866869
/*
@@ -877,25 +880,33 @@ static int ovl_do_copy_up(struct ovl_copy_up_ctx *c)
877880
to_index = true;
878881
}
879882

880-
if (S_ISDIR(c->stat.mode) || c->stat.nlink == 1 || to_index)
883+
if (S_ISDIR(c->stat.mode) || c->stat.nlink == 1 || to_index) {
884+
fh = ovl_get_origin_fh(ofs, origin);
885+
if (IS_ERR(fh))
886+
return PTR_ERR(fh);
887+
888+
/* origin_fh may be NULL */
889+
c->origin_fh = fh;
881890
c->origin = true;
891+
}
882892

883893
if (to_index) {
884894
c->destdir = ovl_indexdir(c->dentry->d_sb);
885-
err = ovl_get_index_name(ofs, c->lowerpath.dentry, &c->destname);
895+
err = ovl_get_index_name(ofs, origin, &c->destname);
886896
if (err)
887-
return err;
897+
goto out_free_fh;
888898
} else if (WARN_ON(!c->parent)) {
889899
/* Disconnected dentry must be copied up to index dir */
890-
return -EIO;
900+
err = -EIO;
901+
goto out_free_fh;
891902
} else {
892903
/*
893904
* Mark parent "impure" because it may now contain non-pure
894905
* upper
895906
*/
896907
err = ovl_set_impure(c->parent, c->destdir);
897908
if (err)
898-
return err;
909+
goto out_free_fh;
899910
}
900911

901912
/* Should we copyup with O_TMPFILE or with workdir? */
@@ -927,6 +938,8 @@ static int ovl_do_copy_up(struct ovl_copy_up_ctx *c)
927938
out:
928939
if (to_index)
929940
kfree(c->destname.name);
941+
out_free_fh:
942+
kfree(fh);
930943
return err;
931944
}
932945

‎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
@@ -632,11 +632,15 @@ struct dentry *ovl_decode_real_fh(struct ovl_fs *ofs, struct ovl_fh *fh,
632632
int ovl_check_origin_fh(struct ovl_fs *ofs, struct ovl_fh *fh, bool connected,
633633
struct dentry *upperdentry, struct ovl_path **stackp);
634634
int ovl_verify_set_fh(struct ovl_fs *ofs, struct dentry *dentry,
635-
enum ovl_xattr ox, struct dentry *real, bool is_upper,
636-
bool set);
635+
enum ovl_xattr ox, const struct ovl_fh *fh,
636+
bool is_upper, bool set);
637+
int ovl_verify_origin_xattr(struct ovl_fs *ofs, struct dentry *dentry,
638+
enum ovl_xattr ox, struct dentry *real,
639+
bool is_upper, bool set);
637640
struct dentry *ovl_index_upper(struct ovl_fs *ofs, struct dentry *index,
638641
bool connected);
639642
int ovl_verify_index(struct ovl_fs *ofs, struct dentry *index);
643+
int ovl_get_index_name_fh(const struct ovl_fh *fh, struct qstr *name);
640644
int ovl_get_index_name(struct ovl_fs *ofs, struct dentry *origin,
641645
struct qstr *name);
642646
struct dentry *ovl_get_index_fh(struct ovl_fs *ofs, struct ovl_fh *fh);
@@ -648,17 +652,24 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
648652
unsigned int flags);
649653
bool ovl_lower_positive(struct dentry *dentry);
650654

655+
static inline int ovl_verify_origin_fh(struct ovl_fs *ofs, struct dentry *upper,
656+
const struct ovl_fh *fh, bool set)
657+
{
658+
return ovl_verify_set_fh(ofs, upper, OVL_XATTR_ORIGIN, fh, false, set);
659+
}
660+
651661
static inline int ovl_verify_origin(struct ovl_fs *ofs, struct dentry *upper,
652662
struct dentry *origin, bool set)
653663
{
654-
return ovl_verify_set_fh(ofs, upper, OVL_XATTR_ORIGIN, origin,
655-
false, set);
664+
return ovl_verify_origin_xattr(ofs, upper, OVL_XATTR_ORIGIN, origin,
665+
false, set);
656666
}
657667

658668
static inline int ovl_verify_upper(struct ovl_fs *ofs, struct dentry *index,
659669
struct dentry *upper, bool set)
660670
{
661-
return ovl_verify_set_fh(ofs, index, OVL_XATTR_UPPER, upper, true, set);
671+
return ovl_verify_origin_xattr(ofs, index, OVL_XATTR_UPPER, upper,
672+
true, set);
662673
}
663674

664675
/* readdir.c */
@@ -823,8 +834,9 @@ int ovl_copy_xattr(struct super_block *sb, const struct path *path, struct dentr
823834
int ovl_set_attr(struct ovl_fs *ofs, struct dentry *upper, struct kstat *stat);
824835
struct ovl_fh *ovl_encode_real_fh(struct ovl_fs *ofs, struct dentry *real,
825836
bool is_upper);
826-
int ovl_set_origin(struct ovl_fs *ofs, struct dentry *lower,
827-
struct dentry *upper);
837+
struct ovl_fh *ovl_get_origin_fh(struct ovl_fs *ofs, struct dentry *origin);
838+
int ovl_set_origin_fh(struct ovl_fs *ofs, const struct ovl_fh *fh,
839+
struct dentry *upper);
828840

829841
/* export.c */
830842
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
@@ -879,15 +879,20 @@ static int ovl_get_indexdir(struct super_block *sb, struct ovl_fs *ofs,
879879
{
880880
struct vfsmount *mnt = ovl_upper_mnt(ofs);
881881
struct dentry *indexdir;
882+
struct dentry *origin = ovl_lowerstack(oe)->dentry;
883+
const struct ovl_fh *fh;
882884
int err;
883885

886+
fh = ovl_get_origin_fh(ofs, origin);
887+
if (IS_ERR(fh))
888+
return PTR_ERR(fh);
889+
884890
err = mnt_want_write(mnt);
885891
if (err)
886-
return err;
892+
goto out_free_fh;
887893

888894
/* Verify lower root is upper root origin */
889-
err = ovl_verify_origin(ofs, upperpath->dentry,
890-
ovl_lowerstack(oe)->dentry, true);
895+
err = ovl_verify_origin_fh(ofs, upperpath->dentry, fh, true);
891896
if (err) {
892897
pr_err("failed to verify upper root origin\n");
893898
goto out;
@@ -919,9 +924,10 @@ static int ovl_get_indexdir(struct super_block *sb, struct ovl_fs *ofs,
919924
* directory entries.
920925
*/
921926
if (ovl_check_origin_xattr(ofs, ofs->indexdir)) {
922-
err = ovl_verify_set_fh(ofs, ofs->indexdir,
923-
OVL_XATTR_ORIGIN,
924-
upperpath->dentry, true, false);
927+
err = ovl_verify_origin_xattr(ofs, ofs->indexdir,
928+
OVL_XATTR_ORIGIN,
929+
upperpath->dentry, true,
930+
false);
925931
if (err)
926932
pr_err("failed to verify index dir 'origin' xattr\n");
927933
}
@@ -939,6 +945,8 @@ static int ovl_get_indexdir(struct super_block *sb, struct ovl_fs *ofs,
939945

940946
out:
941947
mnt_drop_write(mnt);
948+
out_free_fh:
949+
kfree(fh);
942950
return err;
943951
}
944952

‎fs/overlayfs/util.c

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -976,12 +976,18 @@ static void ovl_cleanup_index(struct dentry *dentry)
976976
struct dentry *index = NULL;
977977
struct inode *inode;
978978
struct qstr name = { };
979+
bool got_write = false;
979980
int err;
980981

981982
err = ovl_get_index_name(ofs, lowerdentry, &name);
982983
if (err)
983984
goto fail;
984985

986+
err = ovl_want_write(dentry);
987+
if (err)
988+
goto fail;
989+
990+
got_write = true;
985991
inode = d_inode(upperdentry);
986992
if (!S_ISDIR(inode->i_mode) && inode->i_nlink != 1) {
987993
pr_warn_ratelimited("cleanup linked index (%pd2, ino=%lu, nlink=%u)\n",
@@ -1019,6 +1025,8 @@ static void ovl_cleanup_index(struct dentry *dentry)
10191025
goto fail;
10201026

10211027
out:
1028+
if (got_write)
1029+
ovl_drop_write(dentry);
10221030
kfree(name.name);
10231031
dput(index);
10241032
return;
@@ -1089,6 +1097,8 @@ void ovl_nlink_end(struct dentry *dentry)
10891097
{
10901098
struct inode *inode = d_inode(dentry);
10911099

1100+
ovl_drop_write(dentry);
1101+
10921102
if (ovl_test_flag(OVL_INDEX, inode) && inode->i_nlink == 0) {
10931103
const struct cred *old_cred;
10941104

0 commit comments

Comments
 (0)