BACKPORT: Smack: Rework file hooks

This is one of those cases where you look at code you did
years ago and wonder what you might have been thinking.
There are a number of LSM hooks that work off of file pointers,
and most of them really want the security data from the inode.
Some, however, really want the security context that the process
had when the file was opened. The difference went undetected in
Smack until it started getting used in a real system with real
testing. At that point it was clear that something was amiss.

This patch corrects the misuse of the f_security value in several
of the hooks. The behavior will not usually be any different, as
the process had to be able to open the file in the first place, and
the old check almost always succeeded, as will the new, but for
different reasons.

Thanks to the Samsung Tizen development team that identified this.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
(cherry-picked from upstream 5e7270a6dd14fa6e3bb10128f200305b4a75f350)

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 3d025df93c0894fe0be87d2cfe041b0c5c25a6b9..c02c76fc86492cb52bae3dd18bf7e80836727aac 100644 (file)
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -168,7 +168,7 @@ static int smk_bu_file(struct file *file, int mode, int rc)
 
        smk_bu_mode(mode, acc);
        pr_info("Smack Bringup: (%s %s %s) file=(%s %ld %s) %s\n",
-               sskp->smk_known, (char *)file->f_security, acc,
+               sskp->smk_known, smk_of_inode(inode)->smk_known, acc,
                inode->i_sb->s_id, inode->i_ino, file->f_dentry->d_name.name,
                current->comm);
        return 0;
@@ -1350,6 +1350,9 @@ static int smack_file_permission(struct file *file, int mask)
  * The security blob for a file is a pointer to the master
  * label list, so no allocation is done.
  *
+ * f_security is the owner security information. It
+ * isn't used on file access checks, it's for send_sigio.
+ *
  * Returns 0
  */
 static int smack_file_alloc_security(struct file *file)
@@ -1387,17 +1390,18 @@ static int smack_file_ioctl(struct file *file, unsigned int cmd,
 {
        int rc = 0;
        struct smk_audit_info ad;
+       struct inode *inode = file->f_path.dentry->d_inode;
 
        smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_PATH);
        smk_ad_setfield_u_fs_path(&ad, file->f_path);
 
        if (_IOC_DIR(cmd) & _IOC_WRITE) {
-               rc = smk_curacc(file->f_security, MAY_WRITE, &ad);
+               rc = smk_curacc(smk_of_inode(inode), MAY_WRITE, &ad);
                rc = smk_bu_file(file, MAY_WRITE, rc);
        }
 
        if (rc == 0 && (_IOC_DIR(cmd) & _IOC_READ)) {
-               rc = smk_curacc(file->f_security, MAY_READ, &ad);
+               rc = smk_curacc(smk_of_inode(inode), MAY_READ, &ad);
                rc = smk_bu_file(file, MAY_READ, rc);
        }
 
@@ -1415,10 +1419,11 @@ static int smack_file_lock(struct file *file, unsigned int cmd)
 {
        struct smk_audit_info ad;
        int rc;
+       struct inode *inode = file->f_path.dentry->d_inode;
 
        smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_PATH);
        smk_ad_setfield_u_fs_path(&ad, file->f_path);
-       rc = smk_curacc(file->f_security, MAY_LOCK, &ad);
+       rc = smk_curacc(smk_of_inode(inode), MAY_LOCK, &ad);
        rc = smk_bu_file(file, MAY_LOCK, rc);
        return rc;
 }
@@ -1440,7 +1445,7 @@ static int smack_file_fcntl(struct file *file, unsigned int cmd,
 {
        struct smk_audit_info ad;
        int rc = 0;
-
+       struct inode *inode = file->f_path.dentry->d_inode;
 
        switch (cmd) {
        case F_GETLK:
@@ -1449,14 +1454,14 @@ static int smack_file_fcntl(struct file *file, unsigned int cmd,
        case F_SETLKW:
                smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_PATH);
                smk_ad_setfield_u_fs_path(&ad, file->f_path);
-               rc = smk_curacc(file->f_security, MAY_LOCK, &ad);
+               rc = smk_curacc(smk_of_inode(inode), MAY_LOCK, &ad);
                rc = smk_bu_file(file, MAY_LOCK, rc);
                break;
        case F_SETOWN:
        case F_SETSIG:
                smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_PATH);
                smk_ad_setfield_u_fs_path(&ad, file->f_path);
-               rc = smk_curacc(file->f_security, MAY_WRITE, &ad);
+               rc = smk_curacc(smk_of_inode(inode), MAY_WRITE, &ad);
                rc = smk_bu_file(file, MAY_WRITE, rc);
                break;
        default:
@@ -1586,14 +1591,10 @@ static int smack_file_mmap(struct file *file,
  * smack_file_set_fowner - set the file security blob value
  * @file: object in question
  *
- * Returns 0
- * Further research may be required on this one.
  */
 static int smack_file_set_fowner(struct file *file)
 {
-       struct smack_known *skp = smk_of_current();
-
-       file->f_security = skp;
+       file->f_security = smk_of_current();
        return 0;
 }
 
@@ -1646,6 +1647,7 @@ static int smack_file_receive(struct file *file)
        int rc;
        int may = 0;
        struct smk_audit_info ad;
+       struct inode *inode = file->f_path.dentry->d_inode;
 
        smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_PATH);
        smk_ad_setfield_u_fs_path(&ad, file->f_path);
@@ -1657,7 +1659,7 @@ static int smack_file_receive(struct file *file)
        if (file->f_mode & FMODE_WRITE)
                may |= MAY_WRITE;
 
-       rc = smk_curacc(file->f_security, may, &ad);
+       rc = smk_curacc(smk_of_inode(inode), may, &ad);
        rc = smk_bu_file(file, may, rc);
        return rc;
 }
@@ -1677,21 +1679,17 @@ static int smack_file_receive(struct file *file)
 static int smack_dentry_open(struct file *file, const struct cred *cred)
 {
        struct task_smack *tsp = cred->security;
-       struct inode_smack *isp = file->f_path.dentry->d_inode->i_security;
+       struct inode *inode = file->f_path.dentry->d_inode;
        struct smk_audit_info ad;
        int rc;
 
-       if (smack_privileged(CAP_MAC_OVERRIDE)) {
-               file->f_security = isp->smk_inode;
+       if (smack_privileged(CAP_MAC_OVERRIDE))
                return 0;
-       }
 
        smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_PATH);
        smk_ad_setfield_u_fs_path(&ad, file->f_path);
-       rc = smk_access(tsp->smk_task, isp->smk_inode, MAY_READ, &ad);
+       rc = smk_access(tsp->smk_task, smk_of_inode(inode), MAY_READ, &ad);
        rc = smk_bu_credfile(cred, file, MAY_READ, rc);
-       if (rc == 0)
-               file->f_security = isp->smk_inode;
 
        return rc;
 }