mirror of
https://github.com/rd-stuffs/msm-4.14.git
synced 2025-02-20 11:45:48 +08:00
ANDROID: sdcardfs: Don't use OVERRIDE_CRED macro
The macro hides some control flow, making it easier to run into bugs. bug: 111642636 Change-Id: I37ec207c277d97c4e7f1e8381bc9ae743ad78435 Reported-by: Jann Horn <jannh@google.com> Signed-off-by: Daniel Rosenberg <drosen@google.com>
This commit is contained in:
Daniel Rosenberg
Todd Kjos
parent
6cff87be49
commit
e00c21e88e
@ -118,7 +118,11 @@ static long sdcardfs_unlocked_ioctl(struct file *file, unsigned int cmd,
|
||||
goto out;
|
||||
|
||||
/* save current_cred and override it */
|
||||
OVERRIDE_CRED(sbi, saved_cred, SDCARDFS_I(file_inode(file)));
|
||||
saved_cred = override_fsids(sbi, SDCARDFS_I(file_inode(file))->data);
|
||||
if (!saved_cred) {
|
||||
err = -ENOMEM;
|
||||
goto out;
|
||||
}
|
||||
|
||||
if (lower_file->f_op->unlocked_ioctl)
|
||||
err = lower_file->f_op->unlocked_ioctl(lower_file, cmd, arg);
|
||||
@ -127,7 +131,7 @@ static long sdcardfs_unlocked_ioctl(struct file *file, unsigned int cmd,
|
||||
if (!err)
|
||||
sdcardfs_copy_and_fix_attrs(file_inode(file),
|
||||
file_inode(lower_file));
|
||||
REVERT_CRED(saved_cred);
|
||||
revert_fsids(saved_cred);
|
||||
out:
|
||||
return err;
|
||||
}
|
||||
@ -149,12 +153,16 @@ static long sdcardfs_compat_ioctl(struct file *file, unsigned int cmd,
|
||||
goto out;
|
||||
|
||||
/* save current_cred and override it */
|
||||
OVERRIDE_CRED(sbi, saved_cred, SDCARDFS_I(file_inode(file)));
|
||||
saved_cred = override_fsids(sbi, SDCARDFS_I(file_inode(file))->data);
|
||||
if (!saved_cred) {
|
||||
err = -ENOMEM;
|
||||
goto out;
|
||||
}
|
||||
|
||||
if (lower_file->f_op->compat_ioctl)
|
||||
err = lower_file->f_op->compat_ioctl(lower_file, cmd, arg);
|
||||
|
||||
REVERT_CRED(saved_cred);
|
||||
revert_fsids(saved_cred);
|
||||
out:
|
||||
return err;
|
||||
}
|
||||
@ -241,7 +249,11 @@ static int sdcardfs_open(struct inode *inode, struct file *file)
|
||||
}
|
||||
|
||||
/* save current_cred and override it */
|
||||
OVERRIDE_CRED(sbi, saved_cred, SDCARDFS_I(inode));
|
||||
saved_cred = override_fsids(sbi, SDCARDFS_I(inode)->data);
|
||||
if (!saved_cred) {
|
||||
err = -ENOMEM;
|
||||
goto out_err;
|
||||
}
|
||||
|
||||
file->private_data =
|
||||
kzalloc(sizeof(struct sdcardfs_file_info), GFP_KERNEL);
|
||||
@ -271,7 +283,7 @@ static int sdcardfs_open(struct inode *inode, struct file *file)
|
||||
sdcardfs_copy_and_fix_attrs(inode, sdcardfs_lower_inode(inode));
|
||||
|
||||
out_revert_cred:
|
||||
REVERT_CRED(saved_cred);
|
||||
revert_fsids(saved_cred);
|
||||
out_err:
|
||||
dput(parent);
|
||||
return err;
|
||||
|
||||
@ -22,7 +22,6 @@
|
||||
#include <linux/fs_struct.h>
|
||||
#include <linux/ratelimit.h>
|
||||
|
||||
/* Do not directly use this function. Use OVERRIDE_CRED() instead. */
|
||||
const struct cred *override_fsids(struct sdcardfs_sb_info *sbi,
|
||||
struct sdcardfs_inode_data *data)
|
||||
{
|
||||
@ -50,7 +49,6 @@ const struct cred *override_fsids(struct sdcardfs_sb_info *sbi,
|
||||
return old_cred;
|
||||
}
|
||||
|
||||
/* Do not directly use this function, use REVERT_CRED() instead. */
|
||||
void revert_fsids(const struct cred *old_cred)
|
||||
{
|
||||
const struct cred *cur_cred;
|
||||
@ -78,7 +76,10 @@ static int sdcardfs_create(struct inode *dir, struct dentry *dentry,
|
||||
}
|
||||
|
||||
/* save current_cred and override it */
|
||||
OVERRIDE_CRED(SDCARDFS_SB(dir->i_sb), saved_cred, SDCARDFS_I(dir));
|
||||
saved_cred = override_fsids(SDCARDFS_SB(dir->i_sb),
|
||||
SDCARDFS_I(dir)->data);
|
||||
if (!saved_cred)
|
||||
return -ENOMEM;
|
||||
|
||||
sdcardfs_get_lower_path(dentry, &lower_path);
|
||||
lower_dentry = lower_path.dentry;
|
||||
@ -115,53 +116,11 @@ out:
|
||||
out_unlock:
|
||||
unlock_dir(lower_parent_dentry);
|
||||
sdcardfs_put_lower_path(dentry, &lower_path);
|
||||
REVERT_CRED(saved_cred);
|
||||
revert_fsids(saved_cred);
|
||||
out_eacces:
|
||||
return err;
|
||||
}
|
||||
|
||||
#if 0
|
||||
static int sdcardfs_link(struct dentry *old_dentry, struct inode *dir,
|
||||
struct dentry *new_dentry)
|
||||
{
|
||||
struct dentry *lower_old_dentry;
|
||||
struct dentry *lower_new_dentry;
|
||||
struct dentry *lower_dir_dentry;
|
||||
u64 file_size_save;
|
||||
int err;
|
||||
struct path lower_old_path, lower_new_path;
|
||||
|
||||
OVERRIDE_CRED(SDCARDFS_SB(dir->i_sb));
|
||||
|
||||
file_size_save = i_size_read(d_inode(old_dentry));
|
||||
sdcardfs_get_lower_path(old_dentry, &lower_old_path);
|
||||
sdcardfs_get_lower_path(new_dentry, &lower_new_path);
|
||||
lower_old_dentry = lower_old_path.dentry;
|
||||
lower_new_dentry = lower_new_path.dentry;
|
||||
lower_dir_dentry = lock_parent(lower_new_dentry);
|
||||
|
||||
err = vfs_link(lower_old_dentry, d_inode(lower_dir_dentry),
|
||||
lower_new_dentry, NULL);
|
||||
if (err || !d_inode(lower_new_dentry))
|
||||
goto out;
|
||||
|
||||
err = sdcardfs_interpose(new_dentry, dir->i_sb, &lower_new_path);
|
||||
if (err)
|
||||
goto out;
|
||||
fsstack_copy_attr_times(dir, d_inode(lower_new_dentry));
|
||||
fsstack_copy_inode_size(dir, d_inode(lower_new_dentry));
|
||||
set_nlink(d_inode(old_dentry),
|
||||
sdcardfs_lower_inode(d_inode(old_dentry))->i_nlink);
|
||||
i_size_write(d_inode(new_dentry), file_size_save);
|
||||
out:
|
||||
unlock_dir(lower_dir_dentry);
|
||||
sdcardfs_put_lower_path(old_dentry, &lower_old_path);
|
||||
sdcardfs_put_lower_path(new_dentry, &lower_new_path);
|
||||
REVERT_CRED();
|
||||
return err;
|
||||
}
|
||||
#endif
|
||||
|
||||
static int sdcardfs_unlink(struct inode *dir, struct dentry *dentry)
|
||||
{
|
||||
int err;
|
||||
@ -178,7 +137,10 @@ static int sdcardfs_unlink(struct inode *dir, struct dentry *dentry)
|
||||
}
|
||||
|
||||
/* save current_cred and override it */
|
||||
OVERRIDE_CRED(SDCARDFS_SB(dir->i_sb), saved_cred, SDCARDFS_I(dir));
|
||||
saved_cred = override_fsids(SDCARDFS_SB(dir->i_sb),
|
||||
SDCARDFS_I(dir)->data);
|
||||
if (!saved_cred)
|
||||
return -ENOMEM;
|
||||
|
||||
sdcardfs_get_lower_path(dentry, &lower_path);
|
||||
lower_dentry = lower_path.dentry;
|
||||
@ -209,43 +171,11 @@ out:
|
||||
unlock_dir(lower_dir_dentry);
|
||||
dput(lower_dentry);
|
||||
sdcardfs_put_lower_path(dentry, &lower_path);
|
||||
REVERT_CRED(saved_cred);
|
||||
revert_fsids(saved_cred);
|
||||
out_eacces:
|
||||
return err;
|
||||
}
|
||||
|
||||
#if 0
|
||||
static int sdcardfs_symlink(struct inode *dir, struct dentry *dentry,
|
||||
const char *symname)
|
||||
{
|
||||
int err;
|
||||
struct dentry *lower_dentry;
|
||||
struct dentry *lower_parent_dentry = NULL;
|
||||
struct path lower_path;
|
||||
|
||||
OVERRIDE_CRED(SDCARDFS_SB(dir->i_sb));
|
||||
|
||||
sdcardfs_get_lower_path(dentry, &lower_path);
|
||||
lower_dentry = lower_path.dentry;
|
||||
lower_parent_dentry = lock_parent(lower_dentry);
|
||||
|
||||
err = vfs_symlink(d_inode(lower_parent_dentry), lower_dentry, symname);
|
||||
if (err)
|
||||
goto out;
|
||||
err = sdcardfs_interpose(dentry, dir->i_sb, &lower_path);
|
||||
if (err)
|
||||
goto out;
|
||||
fsstack_copy_attr_times(dir, sdcardfs_lower_inode(dir));
|
||||
fsstack_copy_inode_size(dir, d_inode(lower_parent_dentry));
|
||||
|
||||
out:
|
||||
unlock_dir(lower_parent_dentry);
|
||||
sdcardfs_put_lower_path(dentry, &lower_path);
|
||||
REVERT_CRED();
|
||||
return err;
|
||||
}
|
||||
#endif
|
||||
|
||||
static int touch(char *abs_path, mode_t mode)
|
||||
{
|
||||
struct file *filp = filp_open(abs_path, O_RDWR|O_CREAT|O_EXCL|O_NOFOLLOW, mode);
|
||||
@ -287,7 +217,10 @@ static int sdcardfs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode
|
||||
}
|
||||
|
||||
/* save current_cred and override it */
|
||||
OVERRIDE_CRED(SDCARDFS_SB(dir->i_sb), saved_cred, SDCARDFS_I(dir));
|
||||
saved_cred = override_fsids(SDCARDFS_SB(dir->i_sb),
|
||||
SDCARDFS_I(dir)->data);
|
||||
if (!saved_cred)
|
||||
return -ENOMEM;
|
||||
|
||||
/* check disk space */
|
||||
parent_dentry = dget_parent(dentry);
|
||||
@ -366,13 +299,21 @@ static int sdcardfs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode
|
||||
if (make_nomedia_in_obb ||
|
||||
((pd->perm == PERM_ANDROID)
|
||||
&& (qstr_case_eq(&dentry->d_name, &q_data)))) {
|
||||
REVERT_CRED(saved_cred);
|
||||
OVERRIDE_CRED(SDCARDFS_SB(dir->i_sb), saved_cred, SDCARDFS_I(d_inode(dentry)));
|
||||
revert_fsids(saved_cred);
|
||||
saved_cred = override_fsids(sbi,
|
||||
SDCARDFS_I(d_inode(dentry))->data);
|
||||
if (!saved_cred) {
|
||||
pr_err("sdcardfs: failed to set up .nomedia in %s: %d\n",
|
||||
lower_path.dentry->d_name.name,
|
||||
-ENOMEM);
|
||||
goto out;
|
||||
}
|
||||
set_fs_pwd(current->fs, &lower_path);
|
||||
touch_err = touch(".nomedia", 0664);
|
||||
if (touch_err) {
|
||||
pr_err("sdcardfs: failed to create .nomedia in %s: %d\n",
|
||||
lower_path.dentry->d_name.name, touch_err);
|
||||
lower_path.dentry->d_name.name,
|
||||
touch_err);
|
||||
goto out;
|
||||
}
|
||||
}
|
||||
@ -382,7 +323,7 @@ out:
|
||||
out_unlock:
|
||||
sdcardfs_put_lower_path(dentry, &lower_path);
|
||||
out_revert:
|
||||
REVERT_CRED(saved_cred);
|
||||
revert_fsids(saved_cred);
|
||||
out_eacces:
|
||||
return err;
|
||||
}
|
||||
@ -402,7 +343,10 @@ static int sdcardfs_rmdir(struct inode *dir, struct dentry *dentry)
|
||||
}
|
||||
|
||||
/* save current_cred and override it */
|
||||
OVERRIDE_CRED(SDCARDFS_SB(dir->i_sb), saved_cred, SDCARDFS_I(dir));
|
||||
saved_cred = override_fsids(SDCARDFS_SB(dir->i_sb),
|
||||
SDCARDFS_I(dir)->data);
|
||||
if (!saved_cred)
|
||||
return -ENOMEM;
|
||||
|
||||
/* sdcardfs_get_real_lower(): in case of remove an user's obb dentry
|
||||
* the dentry on the original path should be deleted.
|
||||
@ -427,44 +371,11 @@ static int sdcardfs_rmdir(struct inode *dir, struct dentry *dentry)
|
||||
out:
|
||||
unlock_dir(lower_dir_dentry);
|
||||
sdcardfs_put_real_lower(dentry, &lower_path);
|
||||
REVERT_CRED(saved_cred);
|
||||
revert_fsids(saved_cred);
|
||||
out_eacces:
|
||||
return err;
|
||||
}
|
||||
|
||||
#if 0
|
||||
static int sdcardfs_mknod(struct inode *dir, struct dentry *dentry, umode_t mode,
|
||||
dev_t dev)
|
||||
{
|
||||
int err;
|
||||
struct dentry *lower_dentry;
|
||||
struct dentry *lower_parent_dentry = NULL;
|
||||
struct path lower_path;
|
||||
|
||||
OVERRIDE_CRED(SDCARDFS_SB(dir->i_sb));
|
||||
|
||||
sdcardfs_get_lower_path(dentry, &lower_path);
|
||||
lower_dentry = lower_path.dentry;
|
||||
lower_parent_dentry = lock_parent(lower_dentry);
|
||||
|
||||
err = vfs_mknod(d_inode(lower_parent_dentry), lower_dentry, mode, dev);
|
||||
if (err)
|
||||
goto out;
|
||||
|
||||
err = sdcardfs_interpose(dentry, dir->i_sb, &lower_path);
|
||||
if (err)
|
||||
goto out;
|
||||
fsstack_copy_attr_times(dir, sdcardfs_lower_inode(dir));
|
||||
fsstack_copy_inode_size(dir, d_inode(lower_parent_dentry));
|
||||
|
||||
out:
|
||||
unlock_dir(lower_parent_dentry);
|
||||
sdcardfs_put_lower_path(dentry, &lower_path);
|
||||
REVERT_CRED();
|
||||
return err;
|
||||
}
|
||||
#endif
|
||||
|
||||
/*
|
||||
* The locking rules in sdcardfs_rename are complex. We could use a simpler
|
||||
* superblock-level name-space lock for renames and copy-ups.
|
||||
@ -493,7 +404,10 @@ static int sdcardfs_rename(struct inode *old_dir, struct dentry *old_dentry,
|
||||
}
|
||||
|
||||
/* save current_cred and override it */
|
||||
OVERRIDE_CRED(SDCARDFS_SB(old_dir->i_sb), saved_cred, SDCARDFS_I(new_dir));
|
||||
saved_cred = override_fsids(SDCARDFS_SB(old_dir->i_sb),
|
||||
SDCARDFS_I(new_dir)->data);
|
||||
if (!saved_cred)
|
||||
return -ENOMEM;
|
||||
|
||||
sdcardfs_get_real_lower(old_dentry, &lower_old_path);
|
||||
sdcardfs_get_lower_path(new_dentry, &lower_new_path);
|
||||
@ -540,7 +454,7 @@ out:
|
||||
dput(lower_new_dir_dentry);
|
||||
sdcardfs_put_real_lower(old_dentry, &lower_old_path);
|
||||
sdcardfs_put_lower_path(new_dentry, &lower_new_path);
|
||||
REVERT_CRED(saved_cred);
|
||||
revert_fsids(saved_cred);
|
||||
out_eacces:
|
||||
return err;
|
||||
}
|
||||
@ -660,33 +574,7 @@ static int sdcardfs_permission(struct vfsmount *mnt, struct inode *inode, int ma
|
||||
if (IS_POSIXACL(inode))
|
||||
pr_warn("%s: This may be undefined behavior...\n", __func__);
|
||||
err = generic_permission(&tmp, mask);
|
||||
/* XXX
|
||||
* Original sdcardfs code calls inode_permission(lower_inode,.. )
|
||||
* for checking inode permission. But doing such things here seems
|
||||
* duplicated work, because the functions called after this func,
|
||||
* such as vfs_create, vfs_unlink, vfs_rename, and etc,
|
||||
* does exactly same thing, i.e., they calls inode_permission().
|
||||
* So we just let they do the things.
|
||||
* If there are any security hole, just uncomment following if block.
|
||||
*/
|
||||
#if 0
|
||||
if (!err) {
|
||||
/*
|
||||
* Permission check on lower_inode(=EXT4).
|
||||
* we check it with AID_MEDIA_RW permission
|
||||
*/
|
||||
struct inode *lower_inode;
|
||||
|
||||
OVERRIDE_CRED(SDCARDFS_SB(inode->sb));
|
||||
|
||||
lower_inode = sdcardfs_lower_inode(inode);
|
||||
err = inode_permission(lower_inode, mask);
|
||||
|
||||
REVERT_CRED();
|
||||
}
|
||||
#endif
|
||||
return err;
|
||||
|
||||
}
|
||||
|
||||
static int sdcardfs_setattr_wrn(struct dentry *dentry, struct iattr *ia)
|
||||
@ -764,7 +652,10 @@ static int sdcardfs_setattr(struct vfsmount *mnt, struct dentry *dentry, struct
|
||||
goto out_err;
|
||||
|
||||
/* save current_cred and override it */
|
||||
OVERRIDE_CRED(SDCARDFS_SB(dentry->d_sb), saved_cred, SDCARDFS_I(inode));
|
||||
saved_cred = override_fsids(SDCARDFS_SB(dentry->d_sb),
|
||||
SDCARDFS_I(inode)->data);
|
||||
if (!saved_cred)
|
||||
return -ENOMEM;
|
||||
|
||||
sdcardfs_get_lower_path(dentry, &lower_path);
|
||||
lower_dentry = lower_path.dentry;
|
||||
@ -823,7 +714,7 @@ static int sdcardfs_setattr(struct vfsmount *mnt, struct dentry *dentry, struct
|
||||
|
||||
out:
|
||||
sdcardfs_put_lower_path(dentry, &lower_path);
|
||||
REVERT_CRED(saved_cred);
|
||||
revert_fsids(saved_cred);
|
||||
out_err:
|
||||
return err;
|
||||
}
|
||||
@ -907,13 +798,6 @@ const struct inode_operations sdcardfs_dir_iops = {
|
||||
.setattr = sdcardfs_setattr_wrn,
|
||||
.setattr2 = sdcardfs_setattr,
|
||||
.getattr = sdcardfs_getattr,
|
||||
/* XXX Following operations are implemented,
|
||||
* but FUSE(sdcard) or FAT does not support them
|
||||
* These methods are *NOT* perfectly tested.
|
||||
.symlink = sdcardfs_symlink,
|
||||
.link = sdcardfs_link,
|
||||
.mknod = sdcardfs_mknod,
|
||||
*/
|
||||
};
|
||||
|
||||
const struct inode_operations sdcardfs_main_iops = {
|
||||
|
||||
@ -427,7 +427,12 @@ struct dentry *sdcardfs_lookup(struct inode *dir, struct dentry *dentry,
|
||||
}
|
||||
|
||||
/* save current_cred and override it */
|
||||
OVERRIDE_CRED_PTR(SDCARDFS_SB(dir->i_sb), saved_cred, SDCARDFS_I(dir));
|
||||
saved_cred = override_fsids(SDCARDFS_SB(dir->i_sb),
|
||||
SDCARDFS_I(dir)->data);
|
||||
if (!saved_cred) {
|
||||
ret = ERR_PTR(-ENOMEM);
|
||||
goto out_err;
|
||||
}
|
||||
|
||||
sdcardfs_get_lower_path(parent, &lower_parent_path);
|
||||
|
||||
@ -458,7 +463,7 @@ struct dentry *sdcardfs_lookup(struct inode *dir, struct dentry *dentry,
|
||||
|
||||
out:
|
||||
sdcardfs_put_lower_path(parent, &lower_parent_path);
|
||||
REVERT_CRED(saved_cred);
|
||||
revert_fsids(saved_cred);
|
||||
out_err:
|
||||
dput(parent);
|
||||
return ret;
|
||||
|
||||
@ -88,31 +88,6 @@
|
||||
(x)->i_mode = ((x)->i_mode & S_IFMT) | 0775;\
|
||||
} while (0)
|
||||
|
||||
/* OVERRIDE_CRED() and REVERT_CRED()
|
||||
* OVERRIDE_CRED()
|
||||
* backup original task->cred
|
||||
* and modifies task->cred->fsuid/fsgid to specified value.
|
||||
* REVERT_CRED()
|
||||
* restore original task->cred->fsuid/fsgid.
|
||||
* These two macro should be used in pair, and OVERRIDE_CRED() should be
|
||||
* placed at the beginning of a function, right after variable declaration.
|
||||
*/
|
||||
#define OVERRIDE_CRED(sdcardfs_sbi, saved_cred, info) \
|
||||
do { \
|
||||
saved_cred = override_fsids(sdcardfs_sbi, info->data); \
|
||||
if (!saved_cred) \
|
||||
return -ENOMEM; \
|
||||
} while (0)
|
||||
|
||||
#define OVERRIDE_CRED_PTR(sdcardfs_sbi, saved_cred, info) \
|
||||
do { \
|
||||
saved_cred = override_fsids(sdcardfs_sbi, info->data); \
|
||||
if (!saved_cred) \
|
||||
return ERR_PTR(-ENOMEM); \
|
||||
} while (0)
|
||||
|
||||
#define REVERT_CRED(saved_cred) revert_fsids(saved_cred)
|
||||
|
||||
/* Android 5.0 support */
|
||||
|
||||
/* Permission mode for a specific node. Controls how file permissions
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user