Skip to content

Commit 28fb80f

Browse files
committed
Merge tag 'ovl-update-v2-6.16' of git://git.kernel.org/pub/scm/linux/kernel/git/overlayfs/vfs
Pull overlayfs update from Miklos Szeredi: - Fix a regression in getting the path of an open file (e.g. in /proc/PID/maps) for a nested overlayfs setup (André Almeida) - Support data-only layers and verity in a user namespace (unprivileged composefs use case) - Fix a gcc warning (Kees) - Cleanups * tag 'ovl-update-v2-6.16' of git://git.kernel.org/pub/scm/linux/kernel/git/overlayfs/vfs: ovl: Annotate struct ovl_entry with __counted_by() ovl: Replace offsetof() with struct_size() in ovl_stack_free() ovl: Replace offsetof() with struct_size() in ovl_cache_entry_new() ovl: Check for NULL d_inode() in ovl_dentry_upper() ovl: Use str_on_off() helper in ovl_show_options() ovl: don't require "metacopy=on" for "verity" ovl: relax redirect/metacopy requirements for lower -> data redirect ovl: make redirect/metacopy rejection consistent ovl: Fix nested backing file paths
2 parents 7a912d0 + 6f9ccda commit 28fb80f

File tree

7 files changed

+84
-80
lines changed

7 files changed

+84
-80
lines changed

Documentation/filesystems/overlayfs.rst

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -443,6 +443,13 @@ Only the data of the files in the "data-only" lower layers may be visible
443443
when a "metacopy" file in one of the lower layers above it, has a "redirect"
444444
to the absolute path of the "lower data" file in the "data-only" lower layer.
445445

446+
Instead of explicitly enabling "metacopy=on" it is sufficient to specify at
447+
least one data-only layer to enable redirection of data to a data-only layer.
448+
In this case other forms of metacopy are rejected. Note: this way data-only
449+
layers may be used toghether with "userxattr", in which case careful attention
450+
must be given to privileges needed to change the "user.overlay.redirect" xattr
451+
to prevent misuse.
452+
446453
Since kernel version v6.8, "data-only" lower layers can also be added using
447454
the "datadir+" mount options and the fsconfig syscall from new mount api.
448455
For example::

fs/overlayfs/file.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,8 @@ static struct file *ovl_open_realfile(const struct file *file,
4848
if (!inode_owner_or_capable(real_idmap, realinode))
4949
flags &= ~O_NOATIME;
5050

51-
realfile = backing_file_open(&file->f_path, flags, realpath,
52-
current_cred());
51+
realfile = backing_file_open(file_user_path((struct file *) file),
52+
flags, realpath, current_cred());
5353
}
5454
ovl_revert_creds(old_cred);
5555

fs/overlayfs/namei.c

Lines changed: 60 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
struct ovl_lookup_data {
1818
struct super_block *sb;
19+
struct dentry *dentry;
1920
const struct ovl_layer *layer;
2021
struct qstr name;
2122
bool is_dir;
@@ -24,6 +25,7 @@ struct ovl_lookup_data {
2425
bool stop;
2526
bool last;
2627
char *redirect;
28+
char *upperredirect;
2729
int metacopy;
2830
/* Referring to last redirect xattr */
2931
bool absolute_redirect;
@@ -1024,6 +1026,31 @@ int ovl_verify_lowerdata(struct dentry *dentry)
10241026
return ovl_maybe_validate_verity(dentry);
10251027
}
10261028

1029+
/*
1030+
* Following redirects/metacopy can have security consequences: it's like a
1031+
* symlink into the lower layer without the permission checks.
1032+
*
1033+
* This is only a problem if the upper layer is untrusted (e.g comes from an USB
1034+
* drive). This can allow a non-readable file or directory to become readable.
1035+
*
1036+
* Only following redirects when redirects are enabled disables this attack
1037+
* vector when not necessary.
1038+
*/
1039+
static bool ovl_check_follow_redirect(struct ovl_lookup_data *d)
1040+
{
1041+
struct ovl_fs *ofs = OVL_FS(d->sb);
1042+
1043+
if (d->metacopy && !ofs->config.metacopy) {
1044+
pr_warn_ratelimited("refusing to follow metacopy origin for (%pd2)\n", d->dentry);
1045+
return false;
1046+
}
1047+
if ((d->redirect || d->upperredirect) && !ovl_redirect_follow(ofs)) {
1048+
pr_warn_ratelimited("refusing to follow redirect for (%pd2)\n", d->dentry);
1049+
return false;
1050+
}
1051+
return true;
1052+
}
1053+
10271054
struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
10281055
unsigned int flags)
10291056
{
@@ -1039,20 +1066,22 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
10391066
unsigned int ctr = 0;
10401067
struct inode *inode = NULL;
10411068
bool upperopaque = false;
1042-
char *upperredirect = NULL;
1069+
bool check_redirect = (ovl_redirect_follow(ofs) || ofs->numdatalayer);
10431070
struct dentry *this;
10441071
unsigned int i;
10451072
int err;
10461073
bool uppermetacopy = false;
10471074
int metacopy_size = 0;
10481075
struct ovl_lookup_data d = {
10491076
.sb = dentry->d_sb,
1077+
.dentry = dentry,
10501078
.name = dentry->d_name,
10511079
.is_dir = false,
10521080
.opaque = false,
10531081
.stop = false,
1054-
.last = ovl_redirect_follow(ofs) ? false : !ovl_numlower(poe),
1082+
.last = check_redirect ? false : !ovl_numlower(poe),
10551083
.redirect = NULL,
1084+
.upperredirect = NULL,
10561085
.metacopy = 0,
10571086
};
10581087

@@ -1094,8 +1123,8 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
10941123

10951124
if (d.redirect) {
10961125
err = -ENOMEM;
1097-
upperredirect = kstrdup(d.redirect, GFP_KERNEL);
1098-
if (!upperredirect)
1126+
d.upperredirect = kstrdup(d.redirect, GFP_KERNEL);
1127+
if (!d.upperredirect)
10991128
goto out_put_upper;
11001129
if (d.redirect[0] == '/')
11011130
poe = roe;
@@ -1113,7 +1142,12 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
11131142
for (i = 0; !d.stop && i < ovl_numlower(poe); i++) {
11141143
struct ovl_path lower = ovl_lowerstack(poe)[i];
11151144

1116-
if (!ovl_redirect_follow(ofs))
1145+
if (!ovl_check_follow_redirect(&d)) {
1146+
err = -EPERM;
1147+
goto out_put;
1148+
}
1149+
1150+
if (!check_redirect)
11171151
d.last = i == ovl_numlower(poe) - 1;
11181152
else if (d.is_dir || !ofs->numdatalayer)
11191153
d.last = lower.layer->idx == ovl_numlower(roe);
@@ -1126,13 +1160,6 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
11261160
if (!this)
11271161
continue;
11281162

1129-
if ((uppermetacopy || d.metacopy) && !ofs->config.metacopy) {
1130-
dput(this);
1131-
err = -EPERM;
1132-
pr_warn_ratelimited("refusing to follow metacopy origin for (%pd2)\n", dentry);
1133-
goto out_put;
1134-
}
1135-
11361163
/*
11371164
* If no origin fh is stored in upper of a merge dir, store fh
11381165
* of lower dir and set upper parent "impure".
@@ -1185,23 +1212,6 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
11851212
ctr++;
11861213
}
11871214

1188-
/*
1189-
* Following redirects can have security consequences: it's like
1190-
* a symlink into the lower layer without the permission checks.
1191-
* This is only a problem if the upper layer is untrusted (e.g
1192-
* comes from an USB drive). This can allow a non-readable file
1193-
* or directory to become readable.
1194-
*
1195-
* Only following redirects when redirects are enabled disables
1196-
* this attack vector when not necessary.
1197-
*/
1198-
err = -EPERM;
1199-
if (d.redirect && !ovl_redirect_follow(ofs)) {
1200-
pr_warn_ratelimited("refusing to follow redirect for (%pd2)\n",
1201-
dentry);
1202-
goto out_put;
1203-
}
1204-
12051215
if (d.stop)
12061216
break;
12071217

@@ -1212,10 +1222,16 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
12121222
}
12131223
}
12141224

1215-
/* Defer lookup of lowerdata in data-only layers to first access */
1225+
/*
1226+
* Defer lookup of lowerdata in data-only layers to first access.
1227+
* Don't require redirect=follow and metacopy=on in this case.
1228+
*/
12161229
if (d.metacopy && ctr && ofs->numdatalayer && d.absolute_redirect) {
12171230
d.metacopy = 0;
12181231
ctr++;
1232+
} else if (!ovl_check_follow_redirect(&d)) {
1233+
err = -EPERM;
1234+
goto out_put;
12191235
}
12201236

12211237
/*
@@ -1298,28 +1314,34 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
12981314

12991315
/*
13001316
* It's safe to assign upperredirect here: the previous
1301-
* assignment of happens only if upperdentry is non-NULL, and
1317+
* assignment happens only if upperdentry is non-NULL, and
13021318
* this one only if upperdentry is NULL.
13031319
*/
1304-
upperredirect = ovl_get_redirect_xattr(ofs, &upperpath, 0);
1305-
if (IS_ERR(upperredirect)) {
1306-
err = PTR_ERR(upperredirect);
1307-
upperredirect = NULL;
1320+
d.upperredirect = ovl_get_redirect_xattr(ofs, &upperpath, 0);
1321+
if (IS_ERR(d.upperredirect)) {
1322+
err = PTR_ERR(d.upperredirect);
1323+
d.upperredirect = NULL;
13081324
goto out_free_oe;
13091325
}
1326+
13101327
err = ovl_check_metacopy_xattr(ofs, &upperpath, NULL);
13111328
if (err < 0)
13121329
goto out_free_oe;
1313-
uppermetacopy = err;
1330+
d.metacopy = uppermetacopy = err;
13141331
metacopy_size = err;
1332+
1333+
if (!ovl_check_follow_redirect(&d)) {
1334+
err = -EPERM;
1335+
goto out_free_oe;
1336+
}
13151337
}
13161338

13171339
if (upperdentry || ctr) {
13181340
struct ovl_inode_params oip = {
13191341
.upperdentry = upperdentry,
13201342
.oe = oe,
13211343
.index = index,
1322-
.redirect = upperredirect,
1344+
.redirect = d.upperredirect,
13231345
};
13241346

13251347
/* Store lowerdata redirect for lazy lookup */
@@ -1361,7 +1383,7 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
13611383
kfree(origin_path);
13621384
}
13631385
dput(upperdentry);
1364-
kfree(upperredirect);
1386+
kfree(d.upperredirect);
13651387
out:
13661388
kfree(d.redirect);
13671389
ovl_revert_creds(old_cred);

fs/overlayfs/ovl_entry.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ struct ovl_path {
5151

5252
struct ovl_entry {
5353
unsigned int __numlower;
54-
struct ovl_path __lowerstack[];
54+
struct ovl_path __lowerstack[] __counted_by(__numlower);
5555
};
5656

5757
/* private information held for overlayfs's superblock */

fs/overlayfs/params.c

Lines changed: 6 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -871,37 +871,20 @@ int ovl_fs_params_verify(const struct ovl_fs_context *ctx,
871871
config->uuid = OVL_UUID_NULL;
872872
}
873873

874-
/* Resolve verity -> metacopy dependency */
875-
if (config->verity_mode && !config->metacopy) {
876-
/* Don't allow explicit specified conflicting combinations */
877-
if (set.metacopy) {
878-
pr_err("conflicting options: metacopy=off,verity=%s\n",
879-
ovl_verity_mode(config));
880-
return -EINVAL;
881-
}
882-
/* Otherwise automatically enable metacopy. */
883-
config->metacopy = true;
884-
}
885-
886874
/*
887875
* This is to make the logic below simpler. It doesn't make any other
888876
* difference, since redirect_dir=on is only used for upper.
889877
*/
890878
if (!config->upperdir && config->redirect_mode == OVL_REDIRECT_FOLLOW)
891879
config->redirect_mode = OVL_REDIRECT_ON;
892880

893-
/* Resolve verity -> metacopy -> redirect_dir dependency */
881+
/* metacopy -> redirect_dir dependency */
894882
if (config->metacopy && config->redirect_mode != OVL_REDIRECT_ON) {
895883
if (set.metacopy && set.redirect) {
896884
pr_err("conflicting options: metacopy=on,redirect_dir=%s\n",
897885
ovl_redirect_mode(config));
898886
return -EINVAL;
899887
}
900-
if (config->verity_mode && set.redirect) {
901-
pr_err("conflicting options: verity=%s,redirect_dir=%s\n",
902-
ovl_verity_mode(config), ovl_redirect_mode(config));
903-
return -EINVAL;
904-
}
905888
if (set.redirect) {
906889
/*
907890
* There was an explicit redirect_dir=... that resulted
@@ -970,7 +953,7 @@ int ovl_fs_params_verify(const struct ovl_fs_context *ctx,
970953
}
971954

972955

973-
/* Resolve userxattr -> !redirect && !metacopy && !verity dependency */
956+
/* Resolve userxattr -> !redirect && !metacopy dependency */
974957
if (config->userxattr) {
975958
if (set.redirect &&
976959
config->redirect_mode != OVL_REDIRECT_NOFOLLOW) {
@@ -982,11 +965,6 @@ int ovl_fs_params_verify(const struct ovl_fs_context *ctx,
982965
pr_err("conflicting options: userxattr,metacopy=on\n");
983966
return -EINVAL;
984967
}
985-
if (config->verity_mode) {
986-
pr_err("conflicting options: userxattr,verity=%s\n",
987-
ovl_verity_mode(config));
988-
return -EINVAL;
989-
}
990968
/*
991969
* Silently disable default setting of redirect and metacopy.
992970
* This shall be the default in the future as well: these
@@ -1025,11 +1003,6 @@ int ovl_fs_params_verify(const struct ovl_fs_context *ctx,
10251003
*/
10261004
}
10271005

1028-
if (ctx->nr_data > 0 && !config->metacopy) {
1029-
pr_err("lower data-only dirs require metacopy support.\n");
1030-
return -EINVAL;
1031-
}
1032-
10331006
return 0;
10341007
}
10351008

@@ -1078,17 +1051,16 @@ int ovl_show_options(struct seq_file *m, struct dentry *dentry)
10781051
seq_printf(m, ",redirect_dir=%s",
10791052
ovl_redirect_mode(&ofs->config));
10801053
if (ofs->config.index != ovl_index_def)
1081-
seq_printf(m, ",index=%s", ofs->config.index ? "on" : "off");
1054+
seq_printf(m, ",index=%s", str_on_off(ofs->config.index));
10821055
if (ofs->config.uuid != ovl_uuid_def())
10831056
seq_printf(m, ",uuid=%s", ovl_uuid_mode(&ofs->config));
10841057
if (ofs->config.nfs_export != ovl_nfs_export_def)
1085-
seq_printf(m, ",nfs_export=%s", ofs->config.nfs_export ?
1086-
"on" : "off");
1058+
seq_printf(m, ",nfs_export=%s",
1059+
str_on_off(ofs->config.nfs_export));
10871060
if (ofs->config.xino != ovl_xino_def() && !ovl_same_fs(ofs))
10881061
seq_printf(m, ",xino=%s", ovl_xino_mode(&ofs->config));
10891062
if (ofs->config.metacopy != ovl_metacopy_def)
1090-
seq_printf(m, ",metacopy=%s",
1091-
ofs->config.metacopy ? "on" : "off");
1063+
seq_printf(m, ",metacopy=%s", str_on_off(ofs->config.metacopy));
10921064
if (ofs->config.ovl_volatile)
10931065
seq_puts(m, ",volatile");
10941066
if (ofs->config.userxattr)

fs/overlayfs/readdir.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include <linux/security.h>
1414
#include <linux/cred.h>
1515
#include <linux/ratelimit.h>
16+
#include <linux/overflow.h>
1617
#include "overlayfs.h"
1718

1819
struct ovl_cache_entry {
@@ -147,9 +148,8 @@ static struct ovl_cache_entry *ovl_cache_entry_new(struct ovl_readdir_data *rdd,
147148
u64 ino, unsigned int d_type)
148149
{
149150
struct ovl_cache_entry *p;
150-
size_t size = offsetof(struct ovl_cache_entry, name[len + 1]);
151151

152-
p = kmalloc(size, GFP_KERNEL);
152+
p = kmalloc(struct_size(p, name, len + 1), GFP_KERNEL);
153153
if (!p)
154154
return NULL;
155155

fs/overlayfs/util.c

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#include <linux/uuid.h>
1616
#include <linux/namei.h>
1717
#include <linux/ratelimit.h>
18+
#include <linux/overflow.h>
1819
#include "overlayfs.h"
1920

2021
/* Get write access to upper mnt - may fail if upper sb was remounted ro */
@@ -145,9 +146,9 @@ void ovl_stack_free(struct ovl_path *stack, unsigned int n)
145146

146147
struct ovl_entry *ovl_alloc_entry(unsigned int numlower)
147148
{
148-
size_t size = offsetof(struct ovl_entry, __lowerstack[numlower]);
149-
struct ovl_entry *oe = kzalloc(size, GFP_KERNEL);
149+
struct ovl_entry *oe;
150150

151+
oe = kzalloc(struct_size(oe, __lowerstack, numlower), GFP_KERNEL);
151152
if (oe)
152153
oe->__numlower = numlower;
153154

@@ -305,7 +306,9 @@ enum ovl_path_type ovl_path_realdata(struct dentry *dentry, struct path *path)
305306

306307
struct dentry *ovl_dentry_upper(struct dentry *dentry)
307308
{
308-
return ovl_upperdentry_dereference(OVL_I(d_inode(dentry)));
309+
struct inode *inode = d_inode(dentry);
310+
311+
return inode ? ovl_upperdentry_dereference(OVL_I(inode)) : NULL;
309312
}
310313

311314
struct dentry *ovl_dentry_lower(struct dentry *dentry)

0 commit comments

Comments
 (0)