diff options
| author | Mark Salyzyn <salyzyn@google.com> | 2019-09-13 08:25:31 -0700 |
|---|---|---|
| committer | Mark Salyzyn <salyzyn@google.com> | 2019-09-13 09:31:12 -0700 |
| commit | 72c5343cdbc5ec257e45d5387ed7220065c2ecff (patch) | |
| tree | 778df4bbe4b9b8ce7ee80e7e9f73a08f8f155a40 | |
| parent | 31f2f42f42b8cfd2b6c65950b7127c50bfd5bbe3 (diff) | |
ANDROID: regression introduced override_creds=off
Fixes a regression introduced by the series of commits:
commit 272fcd1ca7ceb252b1c3a2961110c7c1722707cf
("ANDROID: overlayfs: override_creds=off option bypass creator_cred"),
commit aab9adb4b81bcce107dd1783f6ac41dc1abe9f15
("Merge 4.4.179 into android-4.4") that took in an incomplete,
backport of commit 54a07fff4b217607e862b2e1e3fcec32919ad583
("ovl: fix uid/gid when creating over whiteout") (or upstream
commit d0e13f5bbe4be7c8f27736fc40503dcec04b7de0
("ovl: fix uid/gid when creating over whiteout"))
where a crash is observed in ovl_create_or_link() when a
simple re-direction command in vendor directory.
/vendor/bin/<Any test> > /vendor/bin/test_log.txt 2>&1&
After further debugging we see that if the output is redirected to a
file which doesn’t exist we see this stack:
[ 377.382745] ovl_create_or_link+0xac/0x710
[ 377.382745] ovl_create_object+0xb8/0x110
[ 377.382745] ovl_create+0x34/0x40
[ 377.382745] path_openat+0xd44/0x15a8
[ 377.382745] do_filp_open+0x80/0x128
[ 377.382745] do_sys_open+0x140/0x250
[ 377.382745] __arm64_sys_openat+0x2c/0x38
ovl_override_creds returns NULL because the override_cred flag is set
to false. This causes ovl_revert_creds also to fail.
There is another call to check override_cred in override_cred call
which overrides the creds permanently as there no revert_creds
associated. So whenever next commit_cred is called we see the crash
as the credentials are permanently overridden.
Signed-off-by: Mark Salyzyn <salyzyn@google.com>
Tested-by: Rishabh/Jeevan <jshriram@qualcomm.corp-partner.google.com>
Bug: 140816499
Bug: 109821005
Bug: 112955896
Bug: 127298877
Bug: 137541192
Change-Id: Icd0d9be82fc57af5ead1eeab99f79adf3adf62ef
| -rw-r--r-- | fs/overlayfs/dir.c | 17 |
1 files changed, 12 insertions, 5 deletions
diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c index eedacae889b9..b4498b95234c 100644 --- a/fs/overlayfs/dir.c +++ b/fs/overlayfs/dir.c @@ -407,7 +407,7 @@ static int ovl_create_or_link(struct dentry *dentry, int mode, dev_t rdev, if (!ovl_dentry_is_opaque(dentry)) { err = ovl_create_upper(dentry, inode, &stat, link, hardlink); } else { - const struct cred *old_cred; + const struct cred *old_cred, *hold_cred = NULL; struct cred *override_cred; old_cred = ovl_override_creds(dentry->d_sb); @@ -415,15 +415,22 @@ static int ovl_create_or_link(struct dentry *dentry, int mode, dev_t rdev, err = -ENOMEM; override_cred = prepare_creds(); if (override_cred) { - override_cred->fsuid = old_cred->fsuid; - override_cred->fsgid = old_cred->fsgid; - put_cred(override_creds(override_cred)); + const struct cred *our_cred; + + our_cred = old_cred; + if (!our_cred) + our_cred = current_cred(); + override_cred->fsuid = our_cred->fsuid; + override_cred->fsgid = our_cred->fsgid; + hold_cred = override_creds(override_cred); put_cred(override_cred); err = ovl_create_over_whiteout(dentry, inode, &stat, link, hardlink); } - revert_creds(old_cred); + ovl_revert_creds(old_cred ?: hold_cred); + if (old_cred && hold_cred) + put_cred(hold_cred); } if (!err) |