aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDaniel Rosenberg <drosen@google.com>2016-10-26 20:27:20 -0700
committerMister Oyster <oysterized@gmail.com>2017-04-13 12:32:15 +0200
commita2f86e577b06f618ad5b2c4cb2fa58e4eb1a3fcc (patch)
tree8eae9df0733b739ba4cac4d5c75745354eb5bbba
parentbf3b6615b7a5cba4886c5363068434508f8ee8aa (diff)
sdcardfs: Use per mount permissions
This switches sdcardfs over to using permission2. Instead of mounting several sdcardfs instances onto the same underlaying directory, you bind mount a single mount several times, and remount with the options you want. These are stored in the private mount data, allowing you to maintain the same tree, but have different permissions for different mount points. Warning functions have been added for permission, as it should never be called, and the correct behavior is unclear. Change-Id: I841b1d70ec60cf2b866fa48edeb74a0b0f8334f5 Signed-off-by: Daniel Rosenberg <drosen@google.com>
-rwxr-xr-xfs/sdcardfs/derived_perm.c18
-rwxr-xr-xfs/sdcardfs/inode.c127
-rwxr-xr-xfs/sdcardfs/lookup.c4
-rwxr-xr-xfs/sdcardfs/main.c8
-rwxr-xr-xfs/sdcardfs/sdcardfs.h44
5 files changed, 150 insertions, 51 deletions
diff --git a/fs/sdcardfs/derived_perm.c b/fs/sdcardfs/derived_perm.c
index 29a58666b..48380e0ce 100755
--- a/fs/sdcardfs/derived_perm.c
+++ b/fs/sdcardfs/derived_perm.c
@@ -141,13 +141,23 @@ void fixup_perms_recursive(struct dentry *dentry, const char* name, size_t len)
info = SDCARDFS_I(dentry->d_inode);
if (needs_fixup(info->perm)) {
+ /* We need permission to fix up these values.
+ * Since permissions are based of of the mount, and
+ * we are accessing without the mount point, we create
+ * a fake mount with the permissions we will be using.
+ */
+ struct vfsmount fakemnt;
+ struct sdcardfs_vfsmount_options opts;
+ fakemnt.data = &opts;
+ opts.gid = AID_SDCARD_RW;
+ opts.mask = 0;
mutex_lock(&dentry->d_inode->i_mutex);
- child = lookup_one_len(name, dentry, len);
+ child = lookup_one_len2(name, &fakemnt, dentry, len);
mutex_unlock(&dentry->d_inode->i_mutex);
if (!IS_ERR(child)) {
if (child->d_inode) {
get_derived_permission(dentry, child);
- fix_derived_permission(child->d_inode);
+ fixup_tmp_permissions(child->d_inode);
}
dput(child);
}
@@ -172,7 +182,7 @@ void fixup_top_recursive(struct dentry *parent) {
if (dentry->d_inode) {
if (SDCARDFS_I(parent->d_inode)->top != SDCARDFS_I(dentry->d_inode)->top) {
get_derived_permission(parent, dentry);
- fix_derived_permission(dentry->d_inode);
+ fixup_tmp_permissions(dentry->d_inode);
fixup_top_recursive(dentry);
}
}
@@ -202,7 +212,7 @@ inline void update_derived_permission_lock(struct dentry *dentry)
dput(parent);
}
}
- fix_derived_permission(dentry->d_inode);
+ fixup_tmp_permissions(dentry->d_inode);
}
int need_graft_path(struct dentry *dentry)
diff --git a/fs/sdcardfs/inode.c b/fs/sdcardfs/inode.c
index 98c9aec3c..892fe8cfb 100755
--- a/fs/sdcardfs/inode.c
+++ b/fs/sdcardfs/inode.c
@@ -530,7 +530,7 @@ static int sdcardfs_rename(struct inode *old_dir, struct dentry *old_dentry,
/* At this point, not all dentry information has been moved, so
* we pass along new_dentry for the name.*/
get_derived_permission_new(new_dentry->d_parent, old_dentry, new_dentry);
- fix_derived_permission(old_dentry->d_inode);
+ fixup_tmp_permissions(old_dentry->d_inode);
fixup_top_recursive(old_dentry);
out:
unlock_rename(lower_old_dir_dentry, lower_new_dir_dentry);
@@ -613,26 +613,63 @@ static void sdcardfs_put_link(struct dentry *dentry, struct nameidata *nd,
}
#endif
-static int sdcardfs_permission(struct inode *inode, int mask)
+static int sdcardfs_permission_wrn(struct inode *inode, int mask)
+{
+ WARN(1, "sdcardfs does not support permission. Use permission2.\n");
+ return -EINVAL;
+}
+
+void copy_attrs(struct inode *dest, const struct inode *src)
+{
+ dest->i_mode = src->i_mode;
+ dest->i_uid = src->i_uid;
+ dest->i_gid = src->i_gid;
+ dest->i_rdev = src->i_rdev;
+ dest->i_atime = src->i_atime;
+ dest->i_mtime = src->i_mtime;
+ dest->i_ctime = src->i_ctime;
+ dest->i_blkbits = src->i_blkbits;
+ dest->i_flags = src->i_flags;
+#ifdef CONFIG_FS_POSIX_ACL
+ dest->i_acl = src->i_acl;
+#endif
+#ifdef CONFIG_SECURITY
+ dest->i_security = src->i_security;
+#endif
+}
+
+static int sdcardfs_permission(struct vfsmount *mnt, struct inode *inode, int mask)
{
int err;
+ struct inode tmp;
struct inode *top = grab_top(SDCARDFS_I(inode));
- if (!top)
+ if (!top) {
+ release_top(SDCARDFS_I(inode));
+ WARN(1, "Top value was null!\n");
return -EINVAL;
- /* Ensure owner is up to date */
- if (!uid_eq(inode->i_uid, top->i_uid)) {
- SDCARDFS_I(inode)->d_uid = SDCARDFS_I(top)->d_uid;
- fix_derived_permission(inode);
}
- release_top(SDCARDFS_I(inode));
/*
* Permission check on sdcardfs inode.
* Calling process should have AID_SDCARD_RW permission
+ * Since generic_permission only needs i_mode, i_uid,
+ * i_gid, and i_sb, we can create a fake inode to pass
+ * this information down in.
+ *
+ * The underlying code may attempt to take locks in some
+ * cases for features we're not using, but if that changes,
+ * locks must be dealt with to avoid undefined behavior.
*/
- err = generic_permission(inode, mask);
-
+ copy_attrs(&tmp, inode);
+ tmp.i_uid = make_kuid(&init_user_ns, SDCARDFS_I(top)->d_uid);
+ tmp.i_gid = make_kgid(&init_user_ns, get_gid(mnt, SDCARDFS_I(top)));
+ tmp.i_mode = (inode->i_mode & S_IFMT) | get_mode(mnt, SDCARDFS_I(top));
+ release_top(SDCARDFS_I(inode));
+ tmp.i_sb = inode->i_sb;
+ if (IS_POSIXACL(inode))
+ printk(KERN_WARNING "%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
@@ -661,7 +698,13 @@ static int sdcardfs_permission(struct inode *inode, int mask)
}
-static int sdcardfs_setattr(struct dentry *dentry, struct iattr *ia)
+static int sdcardfs_setattr_wrn(struct dentry *dentry, struct iattr *ia)
+{
+ WARN(1, "sdcardfs does not support setattr. User setattr2.\n");
+ return -EINVAL;
+}
+
+static int sdcardfs_setattr(struct vfsmount *mnt, struct dentry *dentry, struct iattr *ia)
{
int err = 0;
struct dentry *lower_dentry;
@@ -671,17 +714,45 @@ static int sdcardfs_setattr(struct dentry *dentry, struct iattr *ia)
struct path lower_path;
struct iattr lower_ia;
struct dentry *parent;
+ struct inode tmp;
+ struct inode *top;
+ const struct cred *saved_cred = NULL;
inode = dentry->d_inode;
+ top = grab_top(SDCARDFS_I(inode));
+
+ if (!top) {
+ release_top(SDCARDFS_I(inode));
+ return -EINVAL;
+ }
+
+ /*
+ * Permission check on sdcardfs inode.
+ * Calling process should have AID_SDCARD_RW permission
+ * Since generic_permission only needs i_mode, i_uid,
+ * i_gid, and i_sb, we can create a fake inode to pass
+ * this information down in.
+ *
+ * The underlying code may attempt to take locks in some
+ * cases for features we're not using, but if that changes,
+ * locks must be dealt with to avoid undefined behavior.
+ *
+ */
+ copy_attrs(&tmp, inode);
+ tmp.i_uid = make_kuid(&init_user_ns, SDCARDFS_I(top)->d_uid);
+ tmp.i_gid = make_kgid(&init_user_ns, get_gid(mnt, SDCARDFS_I(top)));
+ tmp.i_mode = (inode->i_mode & S_IFMT) | get_mode(mnt, SDCARDFS_I(top));
+ tmp.i_size = i_size_read(inode);
+ release_top(SDCARDFS_I(inode));
+ tmp.i_sb = inode->i_sb;
/*
* Check if user has permission to change inode. We don't check if
* this user can change the lower inode: that should happen when
* calling notify_change on the lower inode.
*/
- err = inode_change_ok(inode, ia);
+ err = inode_change_ok(&tmp, ia);
- /* no vfs_XXX operations required, cred overriding will be skipped. wj*/
if (!err) {
/* check the Android group ID */
parent = dget_parent(dentry);
@@ -697,6 +768,9 @@ static int sdcardfs_setattr(struct dentry *dentry, struct iattr *ia)
if (err)
goto out_err;
+ /* save current_cred and override it */
+ OVERRIDE_CRED(SDCARDFS_SB(dentry->d_sb), saved_cred);
+
sdcardfs_get_lower_path(dentry, &lower_path);
lower_dentry = lower_path.dentry;
lower_mnt = lower_path.mnt;
@@ -720,7 +794,7 @@ static int sdcardfs_setattr(struct dentry *dentry, struct iattr *ia)
if (current->mm)
down_write(&current->mm->mmap_sem);
if (ia->ia_valid & ATTR_SIZE) {
- err = inode_newsize_ok(inode, ia->ia_size);
+ err = inode_newsize_ok(&tmp, ia->ia_size);
if (err) {
if (current->mm)
up_write(&current->mm->mmap_sem);
@@ -762,11 +836,12 @@ static int sdcardfs_setattr(struct dentry *dentry, struct iattr *ia)
out:
sdcardfs_put_lower_path(dentry, &lower_path);
+ REVERT_CRED(saved_cred);
out_err:
return err;
}
-static int sdcardfs_fillattr(struct inode *inode, struct kstat *stat)
+static int sdcardfs_fillattr(struct vfsmount *mnt, struct inode *inode, struct kstat *stat)
{
struct sdcardfs_inode_info *info = SDCARDFS_I(inode);
struct inode *top = grab_top(info);
@@ -775,10 +850,10 @@ static int sdcardfs_fillattr(struct inode *inode, struct kstat *stat)
stat->dev = inode->i_sb->s_dev;
stat->ino = inode->i_ino;
- stat->mode = (inode->i_mode & S_IFMT) | get_mode(SDCARDFS_I(top));
+ stat->mode = (inode->i_mode & S_IFMT) | get_mode(mnt, SDCARDFS_I(top));
stat->nlink = inode->i_nlink;
stat->uid = make_kuid(&init_user_ns, SDCARDFS_I(top)->d_uid);
- stat->gid = make_kgid(&init_user_ns, get_gid(SDCARDFS_I(top)));
+ stat->gid = make_kgid(&init_user_ns, get_gid(mnt, SDCARDFS_I(top)));
stat->rdev = inode->i_rdev;
stat->size = i_size_read(inode);
stat->atime = inode->i_atime;
@@ -819,14 +894,14 @@ static int sdcardfs_getattr(struct vfsmount *mnt, struct dentry *dentry,
sdcardfs_copy_and_fix_attrs(inode, lower_inode);
fsstack_copy_inode_size(inode, lower_inode);
- err = sdcardfs_fillattr(inode, stat);
+ err = sdcardfs_fillattr(mnt, inode, stat);
sdcardfs_put_lower_path(dentry, &lower_path);
return err;
}
const struct inode_operations sdcardfs_symlink_iops = {
- .permission = sdcardfs_permission,
- .setattr = sdcardfs_setattr,
+ .permission2 = sdcardfs_permission,
+ .setattr2 = sdcardfs_setattr,
/* XXX Following operations are implemented,
* but FUSE(sdcard) or FAT does not support them
* These methods are *NOT* perfectly tested.
@@ -839,12 +914,14 @@ const struct inode_operations sdcardfs_symlink_iops = {
const struct inode_operations sdcardfs_dir_iops = {
.create = sdcardfs_create,
.lookup = sdcardfs_lookup,
- .permission = sdcardfs_permission,
+ .permission = sdcardfs_permission_wrn,
+ .permission2 = sdcardfs_permission,
.unlink = sdcardfs_unlink,
.mkdir = sdcardfs_mkdir,
.rmdir = sdcardfs_rmdir,
.rename = sdcardfs_rename,
- .setattr = sdcardfs_setattr,
+ .setattr = sdcardfs_setattr_wrn,
+ .setattr2 = sdcardfs_setattr,
.getattr = sdcardfs_getattr,
/* XXX Following operations are implemented,
* but FUSE(sdcard) or FAT does not support them
@@ -856,7 +933,9 @@ const struct inode_operations sdcardfs_dir_iops = {
};
const struct inode_operations sdcardfs_main_iops = {
- .permission = sdcardfs_permission,
- .setattr = sdcardfs_setattr,
+ .permission = sdcardfs_permission_wrn,
+ .permission2 = sdcardfs_permission,
+ .setattr = sdcardfs_setattr_wrn,
+ .setattr2 = sdcardfs_setattr,
.getattr = sdcardfs_getattr,
};
diff --git a/fs/sdcardfs/lookup.c b/fs/sdcardfs/lookup.c
index e5a202bf9..711a24e5c 100755
--- a/fs/sdcardfs/lookup.c
+++ b/fs/sdcardfs/lookup.c
@@ -249,6 +249,7 @@ static struct dentry *__sdcardfs_lookup(struct dentry *dentry,
if (err == -ENOENT) {
struct dentry *child;
struct dentry *match = NULL;
+ mutex_lock(&lower_dir_dentry->d_inode->i_mutex);
spin_lock(&lower_dir_dentry->d_lock);
list_for_each_entry(child, &lower_dir_dentry->d_subdirs, d_child) {
if (child && child->d_inode) {
@@ -259,6 +260,7 @@ static struct dentry *__sdcardfs_lookup(struct dentry *dentry,
}
}
spin_unlock(&lower_dir_dentry->d_lock);
+ mutex_unlock(&lower_dir_dentry->d_inode->i_mutex);
if (match) {
err = vfs_path_lookup(lower_dir_dentry,
lower_dir_mnt,
@@ -394,7 +396,7 @@ struct dentry *sdcardfs_lookup(struct inode *dir, struct dentry *dentry,
sdcardfs_lower_inode(dentry->d_inode));
/* get derived permission */
get_derived_permission(parent, dentry);
- fix_derived_permission(dentry->d_inode);
+ fixup_tmp_permissions(dentry->d_inode);
}
/* update parent directory's atime */
fsstack_copy_attr_atime(parent->d_inode,
diff --git a/fs/sdcardfs/main.c b/fs/sdcardfs/main.c
index a693a3592..8d16e4d16 100755
--- a/fs/sdcardfs/main.c
+++ b/fs/sdcardfs/main.c
@@ -28,7 +28,6 @@ enum {
Opt_fsgid,
Opt_gid,
Opt_debug,
- Opt_lower_fs,
Opt_mask,
Opt_multiuser, // May need?
Opt_userid,
@@ -60,11 +59,9 @@ static int parse_options(struct super_block *sb, char *options, int silent,
opts->fs_low_uid = AID_MEDIA_RW;
opts->fs_low_gid = AID_MEDIA_RW;
vfsopts->mask = 0;
- opts->mask = 0;
opts->multiuser = false;
opts->fs_user_id = 0;
vfsopts->gid = 0;
- opts->gid = 0;
/* by default, 0MB is reserved */
opts->reserved_mb = 0;
@@ -97,7 +94,6 @@ static int parse_options(struct super_block *sb, char *options, int silent,
case Opt_gid:
if (match_int(&args[0], &option))
return 0;
- opts->gid = option;
vfsopts->gid = option;
break;
case Opt_userid:
@@ -108,7 +104,6 @@ static int parse_options(struct super_block *sb, char *options, int silent,
case Opt_mask:
if (match_int(&args[0], &option))
return 0;
- opts->mask = option;
vfsopts->mask = option;
break;
case Opt_multiuser:
@@ -258,6 +253,7 @@ static int sdcardfs_read_super(struct vfsmount *mnt, struct super_block *sb,
printk(KERN_INFO "sdcardfs: dev_name -> %s\n", dev_name);
printk(KERN_INFO "sdcardfs: options -> %s\n", (char *)raw_data);
+ printk(KERN_INFO "sdcardfs: mnt -> %p\n", mnt);
/* parse lower path */
err = kern_path(dev_name, LOOKUP_FOLLOW | LOOKUP_DIRECTORY,
@@ -342,7 +338,7 @@ static int sdcardfs_read_super(struct vfsmount *mnt, struct super_block *sb,
setup_derived_state(sb->s_root->d_inode, PERM_ROOT, sb_info->options.fs_user_id, AID_ROOT, false, sb->s_root->d_inode);
snprintf(sb_info->obbpath_s, PATH_MAX, "%s/Android/obb", dev_name);
}
- fix_derived_permission(sb->s_root->d_inode);
+ fixup_tmp_permissions(sb->s_root->d_inode);
sb_info->sb = sb;
list_add(&sb_info->list, &sdcardfs_super_list);
mutex_unlock(&sdcardfs_super_list_lock);
diff --git a/fs/sdcardfs/sdcardfs.h b/fs/sdcardfs/sdcardfs.h
index 8ed143a16..c68eab5b5 100755
--- a/fs/sdcardfs/sdcardfs.h
+++ b/fs/sdcardfs/sdcardfs.h
@@ -68,11 +68,18 @@
#define AID_PACKAGE_INFO 1027
-#define fix_derived_permission(x) \
+
+/*
+ * Permissions are handled by our permission function.
+ * We don't want anyone who happens to look at our inode value to prematurely
+ * block access, so store more permissive values. These are probably never
+ * used.
+ */
+#define fixup_tmp_permissions(x) \
do { \
(x)->i_uid = SDCARDFS_I(x)->d_uid; \
- (x)->i_gid = get_gid(SDCARDFS_I(x)); \
- (x)->i_mode = ((x)->i_mode & S_IFMT) | get_mode(SDCARDFS_I(x));\
+ (x)->i_gid = AID_SDCARD_RW; \
+ (x)->i_mode = ((x)->i_mode & S_IFMT) | 0775;\
} while (0)
@@ -187,8 +194,6 @@ struct sdcardfs_mount_options {
uid_t fs_low_uid;
gid_t fs_low_gid;
userid_t fs_user_id;
- gid_t gid;
- mode_t mask;
bool multiuser;
unsigned int reserved_mb;
};
@@ -360,9 +365,10 @@ static inline void release_top(struct sdcardfs_inode_info *info)
iput(info->top);
}
-static inline int get_gid(struct sdcardfs_inode_info *info) {
- struct sdcardfs_sb_info *sb_info = SDCARDFS_SB(info->vfs_inode.i_sb);
- if (sb_info->options.gid == AID_SDCARD_RW) {
+static inline int get_gid(struct vfsmount *mnt, struct sdcardfs_inode_info *info) {
+ struct sdcardfs_vfsmount_options *opts = mnt->data;
+
+ if (opts->gid == AID_SDCARD_RW) {
/* As an optimization, certain trusted system components only run
* as owner but operate across all users. Since we're now handing
* out the sdcard_rw GID only to trusted apps, we're okay relaxing
@@ -370,14 +376,15 @@ static inline int get_gid(struct sdcardfs_inode_info *info) {
* assigned to app directories are still multiuser aware. */
return AID_SDCARD_RW;
} else {
- return multiuser_get_uid(info->userid, sb_info->options.gid);
+ return multiuser_get_uid(info->userid, opts->gid);
}
}
-static inline int get_mode(struct sdcardfs_inode_info *info) {
+static inline int get_mode(struct vfsmount *mnt, struct sdcardfs_inode_info *info) {
int owner_mode;
int filtered_mode;
- struct sdcardfs_sb_info * sb_info = SDCARDFS_SB(info->vfs_inode.i_sb);
- int visible_mode = 0775 & ~sb_info->options.mask;
+ struct sdcardfs_vfsmount_options *opts = mnt->data;
+ int visible_mode = 0775 & ~opts->mask;
+
if (info->perm == PERM_PRE_ROOT) {
/* Top of multi-user view should always be visible to ensure
@@ -387,7 +394,7 @@ static inline int get_mode(struct sdcardfs_inode_info *info) {
/* Block "other" access to Android directories, since only apps
* belonging to a specific user should be in there; we still
* leave +x open for the default view. */
- if (sb_info->options.gid == AID_SDCARD_RW) {
+ if (opts->gid == AID_SDCARD_RW) {
visible_mode = visible_mode & ~0006;
} else {
visible_mode = visible_mode & ~0007;
@@ -553,12 +560,17 @@ static inline int check_min_free_space(struct dentry *dentry, size_t size, int d
return 1;
}
-/* Copies attrs and maintains sdcardfs managed attrs */
+/*
+ * Copies attrs and maintains sdcardfs managed attrs
+ * Since our permission check handles all special permissions, set those to be open
+ */
static inline void sdcardfs_copy_and_fix_attrs(struct inode *dest, const struct inode *src)
{
- dest->i_mode = (src->i_mode & S_IFMT) | get_mode(SDCARDFS_I(dest));
+
+ dest->i_mode = (src->i_mode & S_IFMT) | S_IRWXU | S_IRWXG |
+ S_IROTH | S_IXOTH; /* 0775 */
dest->i_uid = SDCARDFS_I(dest)->d_uid;
- dest->i_gid = get_gid(SDCARDFS_I(dest));
+ dest->i_gid = AID_SDCARD_RW;
dest->i_rdev = src->i_rdev;
dest->i_atime = src->i_atime;
dest->i_mtime = src->i_mtime;