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

[Casey Schaufler <casey@schaufler-ca.com>]

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 <casey@schaufler-ca.com>
>
> *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:
> > 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?
>
> >
> > Signed-off-by: Vishal Goel <vishal.goel@samsung.com>
> > ---
> >  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;
>