Re: [PATCH 1/1] Smack:- Use overlay inode label in smack_inode_copy_up()

On 9/20/2021 1:08 AM, Vishal Goel wrote:

Hi,

 

Please find below the test binary code and steps to reproduce:-

Excellent. I'll verify the correction, and if all seems sane, include it
for 5.16. Thank you.

 void main()
{
        int fd,pid;
        char cmd[50];

        pid = getpid();
        sprintf(cmd,"cat /proc/%d/attr/current",pid);
        system(cmd);
        fd = open("/test_dir/smack_test/tmp/test_file", O_CREAT | O_RDWR, S_IWUSR | S_IRUSR);

        if(fd != -1) {
                close(fd);
        }
}

*Steps:-*

####### Check default smack labels on files/directories present in the image
~$ chsmack /test_dir/smack_test/                                                                                                                                                         
/test_dir/smack_test/ access="!"

~$ chsmack /test_dir/smack_test/tmp/                                                                                                                                                  
/test_dir/smack_test/tmp/ access="_"

 


####### Flash the image on target/board and reboot

sh-3.2# mount | grep overlay                                                                                                                                                              
overlay on / type overlay (rw,relatime,lowerdir=/,upperdir=/opt/overlay/upperdir,workdir=/opt/overlay/workdir)

 

####### Check the smack labels

sh-3.2# chsmack /test_dir/smack_test/
/test_dir/smack_test/ access="!"
sh-3.2# chsmack /test_dir/smack_test/tmp
/test_dir/smack_test/tmp access="_"                      ====> Same label is present

 

####### Run test binary to create a new file under "/test_dir/smack_test/tmp" directory

During inode creation, smack_inode_copy_up() function is called for each of the directory present in path.
After that "smack_inode_init_security()" is called for initializing the corresponding overlay inode entry.
During initialization of "tmp", parent inode label is used which is "!" in this case.

 

sh-3.2# ./test_bin
Test_Label                                                        ===> Process label
 

####### Reboot the target/board

sh-3.2# chsmack /test_dir/smack_test/                                                                                                                                                         
/test_dir/smack_test/ access="!"

sh-3.2# chsmack /test_dir/smack_test/tmp/                                                                                                                                                  
/test_dir/smack_test/tmp/ access="!"                    ====> Label has been changed from "!" to "_"

 

Thanks & Regards

Vishal Goel

--------- *Original Message* ---------

*Sender* : Casey Schaufler [off-list ref]

*Date* : 2021-09-18 01:32 (GMT+9)

*Title* : Re: [PATCH 1/1] Smack:- Use overlay inode label in smack_inode_copy_up()

 

On 9/17/2021 12:38 AM, Vishal Goel wrote:

quoted

Currently in "smack_inode_copy_up()" function, process label is
changed with the label on parent inode. Due to which,
process is assigned directory label and whatever file or directory
created by the process are also getting directory label
which is wrong label.

Changes has been done to use label of overlay inode instead
of parent inode.

Do you have a test case for this change?

quoted

Signed-off-by: Vishal Goel <redacted>
---
 security/smack/smack_lsm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index cacbe7518..91e50e5cb 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c

@@ -4634,7 +4634,7 @@ static int smack_inode_copy_up(struct dentry *dentry, struct cred **new)
 	/*
 	 * Get label from overlay inode and set it in create_sid
 	 */
-	isp = smack_inode(d_inode(dentry->d_parent));
+	isp = smack_inode(d_inode(dentry));
 	skp = isp->smk_inode;
 	tsp->smk_task = skp;
 	*new = new_creds;