diff mbox series

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

Message ID 1631864294-25794-1-git-send-email-vishal.goel@samsung.com (mailing list archive)
State New, archived
Headers show
Series [1/1] Smack:- Use overlay inode label in smack_inode_copy_up() | expand

Commit Message

Vishal Goel Sept. 17, 2021, 7:38 a.m. UTC
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.

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

Comments

Casey Schaufler Sept. 17, 2021, 4:32 p.m. UTC | #1
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;
Casey Schaufler Sept. 20, 2021, 2:55 p.m. UTC | #2
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;
>
diff mbox series

Patch

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;