ANDROID: sdcardfs: Use per mount permissions
authorDaniel Rosenberg <drosen@google.com>
Thu, 27 Oct 2016 03:27:20 +0000 (20:27 -0700)
committerAmit Pundir <amit.pundir@linaro.org>
Fri, 3 Feb 2017 09:34:29 +0000 (15:04 +0530)
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>
fs/sdcardfs/derived_perm.c
fs/sdcardfs/inode.c
fs/sdcardfs/lookup.c
fs/sdcardfs/main.c
fs/sdcardfs/sdcardfs.h

index 89daf69efbaa45d27425060358228d5d0d18cf56..066edbbb6ad6285ebf5599fbecd9ca56e25d76f3 100644 (file)
@@ -141,13 +141,23 @@ void fixup_perms_recursive(struct dentry *dentry, const char* name, size_t len)
        info = SDCARDFS_I(d_inode(dentry));
 
        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(&d_inode(dentry)->i_mutex);
-               child = lookup_one_len(name, dentry, len);
+               child = lookup_one_len2(name, &fakemnt, dentry, len);
                mutex_unlock(&d_inode(dentry)->i_mutex);
                if (!IS_ERR(child)) {
-                       if (child->d_inode) {
+                       if (d_inode(child)) {
                                get_derived_permission(dentry, child);
-                               fix_derived_permission(d_inode(child));
+                               fixup_tmp_permissions(d_inode(child));
                        }
                        dput(child);
                }
@@ -172,7 +182,7 @@ void fixup_top_recursive(struct dentry *parent) {
                if (d_inode(dentry)) {
                        if (SDCARDFS_I(d_inode(parent))->top != SDCARDFS_I(d_inode(dentry))->top) {
                                get_derived_permission(parent, dentry);
-                               fix_derived_permission(d_inode(dentry));
+                               fixup_tmp_permissions(d_inode(dentry));
                                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(d_inode(dentry));
 }
 
 int need_graft_path(struct dentry *dentry)
index dc64c9e2f5e78e2758f6ed7ce14054df4de9d702..76a6e8ad07361189b3bfa6827e08815ecc8f9c33 100644 (file)
@@ -531,7 +531,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(d_inode(old_dentry));
+       fixup_tmp_permissions(d_inode(old_dentry));
        fixup_top_recursive(old_dentry);
 out:
        unlock_rename(lower_old_dir_dentry, lower_new_dir_dentry);
@@ -601,26 +601,63 @@ static const char *sdcardfs_follow_link(struct dentry *dentry, void **cookie)
 }
 #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
@@ -649,7 +686,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;
        struct dentry *lower_dentry;
@@ -659,17 +702,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 = d_inode(dentry);
+       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);
@@ -685,6 +756,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;
@@ -708,7 +782,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);
@@ -750,11 +824,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);
@@ -763,10 +838,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;
@@ -807,14 +882,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.
@@ -827,12 +902,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
@@ -844,7 +921,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,
 };
index c74a7d1bc18e131c397318cf4f8d1f0a74212a58..00a711ec2733ecf8f395766d3ca4dc6f4b4ad1b9 100644 (file)
@@ -244,6 +244,7 @@ static struct dentry *__sdcardfs_lookup(struct dentry *dentry,
        if (err == -ENOENT) {
                struct dentry *child;
                struct dentry *match = NULL;
+               mutex_lock(&d_inode(lower_dir_dentry)->i_mutex);
                spin_lock(&lower_dir_dentry->d_lock);
                list_for_each_entry(child, &lower_dir_dentry->d_subdirs, d_child) {
                        if (child && d_inode(child)) {
@@ -254,6 +255,7 @@ static struct dentry *__sdcardfs_lookup(struct dentry *dentry,
                        }
                }
                spin_unlock(&lower_dir_dentry->d_lock);
+               mutex_unlock(&d_inode(lower_dir_dentry)->i_mutex);
                if (match) {
                        err = vfs_path_lookup(lower_dir_dentry,
                                                lower_dir_mnt,
@@ -389,7 +391,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(d_inode(dentry));
        }
        /* update parent directory's atime */
        fsstack_copy_attr_atime(parent->d_inode,
index 5400e7e63d27a37f2f40e80783d383d9b7b5f411..eec10ccacd9982cc3a117b56892a4ebbe77af158 100644 (file)
@@ -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(d_inode(sb->s_root), PERM_ROOT, sb_info->options.fs_user_id, AID_ROOT, false, d_inode(sb->s_root));
                snprintf(sb_info->obbpath_s, PATH_MAX, "%s/Android/obb", dev_name);
        }
-       fix_derived_permission(sb->s_root->d_inode);
+       fixup_tmp_permissions(d_inode(sb->s_root));
        sb_info->sb = sb;
        list_add(&sb_info->list, &sdcardfs_super_list);
        mutex_unlock(&sdcardfs_super_list_lock);
index 22ef29857022a2d5f10c602256df57f8f4f55238..b03130329014c7bc75b4a04e5f240d8eafa905dc 100644 (file)
 
 #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 = make_kuid(&init_user_ns, SDCARDFS_I(x)->d_uid);    \
-               (x)->i_gid = make_kgid(&init_user_ns, get_gid(SDCARDFS_I(x)));  \
-               (x)->i_mode = ((x)->i_mode & S_IFMT) | get_mode(SDCARDFS_I(x));\
+               (x)->i_gid = make_kgid(&init_user_ns, AID_SDCARD_RW);   \
+               (x)->i_mode = ((x)->i_mode & S_IFMT) | 0775;\
        } while (0)
 
-
 /* OVERRIDE_CRED() and REVERT_CRED()
  *     OVERRID_CRED()
  *             backup original task->cred
@@ -187,8 +193,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 +364,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 +375,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 +393,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 +559,16 @@ 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 = make_kuid(&init_user_ns, SDCARDFS_I(dest)->d_uid);
-       dest->i_gid = make_kgid(&init_user_ns, get_gid(SDCARDFS_I(dest)));
+       dest->i_gid = make_kgid(&init_user_ns, AID_SDCARD_RW);
        dest->i_rdev = src->i_rdev;
        dest->i_atime = src->i_atime;
        dest->i_mtime = src->i_mtime;