All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] selinux: remove redundant msg_msg_alloc_security
@ 2020-01-10  9:58 Huaisheng Ye
  2020-01-10 15:13 ` Stephen Smalley
  2020-01-10 16:43 ` Paul Moore
  0 siblings, 2 replies; 7+ messages in thread
From: Huaisheng Ye @ 2020-01-10  9:58 UTC (permalink / raw)
  To: paul, sds, eparis, jmorris, serge
  Cc: tyu1, linux-security-module, selinux, linux-kernel, Huaisheng Ye

From: Huaisheng Ye <yehs1@lenovo.com>

selinux_msg_msg_alloc_security only calls msg_msg_alloc_security but
do nothing else. And also msg_msg_alloc_security is just used by the
former.

Remove the redundant function to simplify the code.

Signed-off-by: Huaisheng Ye <yehs1@lenovo.com>
---
 security/selinux/hooks.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 9625b99..fb1b9da 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -5882,16 +5882,6 @@ static void ipc_init_security(struct ipc_security_struct *isec, u16 sclass)
 	isec->sid = current_sid();
 }
 
-static int msg_msg_alloc_security(struct msg_msg *msg)
-{
-	struct msg_security_struct *msec;
-
-	msec = selinux_msg_msg(msg);
-	msec->sid = SECINITSID_UNLABELED;
-
-	return 0;
-}
-
 static int ipc_has_perm(struct kern_ipc_perm *ipc_perms,
 			u32 perms)
 {
@@ -5910,7 +5900,12 @@ static int ipc_has_perm(struct kern_ipc_perm *ipc_perms,
 
 static int selinux_msg_msg_alloc_security(struct msg_msg *msg)
 {
-	return msg_msg_alloc_security(msg);
+	struct msg_security_struct *msec;
+
+	msec = selinux_msg_msg(msg);
+	msec->sid = SECINITSID_UNLABELED;
+
+	return 0;
 }
 
 /* message queue security operations */
-- 
1.8.3.1



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] selinux: remove redundant msg_msg_alloc_security
  2020-01-10  9:58 [PATCH] selinux: remove redundant msg_msg_alloc_security Huaisheng Ye
@ 2020-01-10 15:13 ` Stephen Smalley
  2020-01-10 16:44   ` Casey Schaufler
  2020-01-10 16:50   ` Paul Moore
  2020-01-10 16:43 ` Paul Moore
  1 sibling, 2 replies; 7+ messages in thread
From: Stephen Smalley @ 2020-01-10 15:13 UTC (permalink / raw)
  To: Huaisheng Ye, paul, eparis, jmorris, serge
  Cc: tyu1, linux-security-module, selinux, linux-kernel, Huaisheng Ye

On 1/10/20 4:58 AM, Huaisheng Ye wrote:
> From: Huaisheng Ye <yehs1@lenovo.com>
> 
> selinux_msg_msg_alloc_security only calls msg_msg_alloc_security but
> do nothing else. And also msg_msg_alloc_security is just used by the
> former.
> 
> Remove the redundant function to simplify the code.

This seems to also be true of other _alloc_security functions, probably 
due to historical reasons.  Further, at least some of these functions no 
longer perform any allocation; they are just initialization functions 
now that allocation has been taken to the LSM framework, so possibly 
could be renamed and made to return void at some point.

> 
> Signed-off-by: Huaisheng Ye <yehs1@lenovo.com>

Acked-by: Stephen Smalley <sds@tycho.nsa.gov>

> ---
>   security/selinux/hooks.c | 17 ++++++-----------
>   1 file changed, 6 insertions(+), 11 deletions(-)
> 
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 9625b99..fb1b9da 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -5882,16 +5882,6 @@ static void ipc_init_security(struct ipc_security_struct *isec, u16 sclass)
>   	isec->sid = current_sid();
>   }
>   
> -static int msg_msg_alloc_security(struct msg_msg *msg)
> -{
> -	struct msg_security_struct *msec;
> -
> -	msec = selinux_msg_msg(msg);
> -	msec->sid = SECINITSID_UNLABELED;
> -
> -	return 0;
> -}
> -
>   static int ipc_has_perm(struct kern_ipc_perm *ipc_perms,
>   			u32 perms)
>   {
> @@ -5910,7 +5900,12 @@ static int ipc_has_perm(struct kern_ipc_perm *ipc_perms,
>   
>   static int selinux_msg_msg_alloc_security(struct msg_msg *msg)
>   {
> -	return msg_msg_alloc_security(msg);
> +	struct msg_security_struct *msec;
> +
> +	msec = selinux_msg_msg(msg);
> +	msec->sid = SECINITSID_UNLABELED;
> +
> +	return 0;
>   }
>   
>   /* message queue security operations */
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] selinux: remove redundant msg_msg_alloc_security
  2020-01-10  9:58 [PATCH] selinux: remove redundant msg_msg_alloc_security Huaisheng Ye
  2020-01-10 15:13 ` Stephen Smalley
@ 2020-01-10 16:43 ` Paul Moore
  1 sibling, 0 replies; 7+ messages in thread
From: Paul Moore @ 2020-01-10 16:43 UTC (permalink / raw)
  To: Huaisheng Ye
  Cc: Stephen Smalley, Eric Paris, James Morris, Serge Hallyn, tyu1,
	linux-security-module, selinux, linux-kernel, Huaisheng Ye

On Fri, Jan 10, 2020 at 4:59 AM Huaisheng Ye <yehs2007@zoho.com> wrote:
> From: Huaisheng Ye <yehs1@lenovo.com>
>
> selinux_msg_msg_alloc_security only calls msg_msg_alloc_security but
> do nothing else. And also msg_msg_alloc_security is just used by the
> former.
>
> Remove the redundant function to simplify the code.
>
> Signed-off-by: Huaisheng Ye <yehs1@lenovo.com>
> ---
>  security/selinux/hooks.c | 17 ++++++-----------
>  1 file changed, 6 insertions(+), 11 deletions(-)

Merged into selinux/next, thanks!

-- 
paul moore
www.paul-moore.com

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] selinux: remove redundant msg_msg_alloc_security
  2020-01-10 15:13 ` Stephen Smalley
@ 2020-01-10 16:44   ` Casey Schaufler
  2020-01-10 16:50   ` Paul Moore
  1 sibling, 0 replies; 7+ messages in thread
From: Casey Schaufler @ 2020-01-10 16:44 UTC (permalink / raw)
  To: Stephen Smalley, Huaisheng Ye, paul, eparis, jmorris, serge
  Cc: tyu1, linux-security-module, selinux, linux-kernel, Huaisheng Ye,
	Casey Schaufler

On 1/10/2020 7:13 AM, Stephen Smalley wrote:
> On 1/10/20 4:58 AM, Huaisheng Ye wrote:
>> From: Huaisheng Ye <yehs1@lenovo.com>
>>
>> selinux_msg_msg_alloc_security only calls msg_msg_alloc_security but
>> do nothing else. And also msg_msg_alloc_security is just used by the
>> former.
>>
>> Remove the redundant function to simplify the code.
>
> This seems to also be true of other _alloc_security functions, probably due to historical reasons.  Further, at least some of these functions no longer perform any allocation; they are just initialization functions now that allocation has been taken to the LSM framework, so possibly could be renamed and made to return void at some point.

That's something I've been eyeing. I'm not 100% sure that no module will
ever fail doing the new style initialization. The int to void and name
change will probably happen after the next round of modules come in and
we can see that failure of initialization isn't a rational situation.

>
>>
>> Signed-off-by: Huaisheng Ye <yehs1@lenovo.com>
>
> Acked-by: Stephen Smalley <sds@tycho.nsa.gov>
>
>> ---
>>   security/selinux/hooks.c | 17 ++++++-----------
>>   1 file changed, 6 insertions(+), 11 deletions(-)
>>
>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>> index 9625b99..fb1b9da 100644
>> --- a/security/selinux/hooks.c
>> +++ b/security/selinux/hooks.c
>> @@ -5882,16 +5882,6 @@ static void ipc_init_security(struct ipc_security_struct *isec, u16 sclass)
>>       isec->sid = current_sid();
>>   }
>>   -static int msg_msg_alloc_security(struct msg_msg *msg)
>> -{
>> -    struct msg_security_struct *msec;
>> -
>> -    msec = selinux_msg_msg(msg);
>> -    msec->sid = SECINITSID_UNLABELED;
>> -
>> -    return 0;
>> -}
>> -
>>   static int ipc_has_perm(struct kern_ipc_perm *ipc_perms,
>>               u32 perms)
>>   {
>> @@ -5910,7 +5900,12 @@ static int ipc_has_perm(struct kern_ipc_perm *ipc_perms,
>>     static int selinux_msg_msg_alloc_security(struct msg_msg *msg)
>>   {
>> -    return msg_msg_alloc_security(msg);
>> +    struct msg_security_struct *msec;
>> +
>> +    msec = selinux_msg_msg(msg);
>> +    msec->sid = SECINITSID_UNLABELED;
>> +
>> +    return 0;
>>   }
>>     /* message queue security operations */
>>
>
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] selinux: remove redundant msg_msg_alloc_security
  2020-01-10 15:13 ` Stephen Smalley
  2020-01-10 16:44   ` Casey Schaufler
@ 2020-01-10 16:50   ` Paul Moore
  2020-01-12 15:45     ` [External] " Huaisheng HS1 Ye
  1 sibling, 1 reply; 7+ messages in thread
From: Paul Moore @ 2020-01-10 16:50 UTC (permalink / raw)
  To: Stephen Smalley
  Cc: Huaisheng Ye, Eric Paris, James Morris, Serge Hallyn, tyu1,
	linux-security-module, selinux, linux-kernel, Huaisheng Ye

On Fri, Jan 10, 2020 at 10:13 AM Stephen Smalley <sds@tycho.nsa.gov> wrote:
> On 1/10/20 4:58 AM, Huaisheng Ye wrote:
> > From: Huaisheng Ye <yehs1@lenovo.com>
> >
> > selinux_msg_msg_alloc_security only calls msg_msg_alloc_security but
> > do nothing else. And also msg_msg_alloc_security is just used by the
> > former.
> >
> > Remove the redundant function to simplify the code.
>
> This seems to also be true of other _alloc_security functions, probably
> due to historical reasons.  Further, at least some of these functions no
> longer perform any allocation; they are just initialization functions
> now that allocation has been taken to the LSM framework, so possibly
> could be renamed and made to return void at some point.

I've noticed the same thing on a few occasions, I've just never
bothered to put the fixes into a patch.  We might as well do that now,
at least for the redundant code bits; I'll leave the return code issue
for another time as that would cross LSM boundaries and that really
isn't appropriate in the -rc5 timeframe IMHO.

I'll put something together once I finish up the patch/review backlog
from the past few days.  Looking quickly with a regex, it would appear
that inode_alloc_security(), file_alloc_security(), and
superblock_alloc_security() are all candidates.  While not an
allocator, we can probably get rid of inode_doinit() as well.

-- 
paul moore
www.paul-moore.com

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [External]  Re: [PATCH] selinux: remove redundant msg_msg_alloc_security
  2020-01-10 16:50   ` Paul Moore
@ 2020-01-12 15:45     ` Huaisheng HS1 Ye
  0 siblings, 0 replies; 7+ messages in thread
From: Huaisheng HS1 Ye @ 2020-01-12 15:45 UTC (permalink / raw)
  To: Paul Moore, Stephen Smalley
  Cc: Huaisheng Ye, Eric Paris, James Morris, Serge Hallyn,
	Tzu ting Yu1, linux-security-module, selinux, linux-kernel


> -----Original Message-----
> From: Paul Moore <paul@paul-moore.com>
> Sent: Saturday, January 11, 2020 12:50 AM
> On Fri, Jan 10, 2020 at 10:13 AM Stephen Smalley <sds@tycho.nsa.gov> wrote:
> > On 1/10/20 4:58 AM, Huaisheng Ye wrote:
> > > From: Huaisheng Ye <yehs1@lenovo.com>
> > >
> > > selinux_msg_msg_alloc_security only calls msg_msg_alloc_security but
> > > do nothing else. And also msg_msg_alloc_security is just used by the
> > > former.
> > >
> > > Remove the redundant function to simplify the code.
> >
> > This seems to also be true of other _alloc_security functions,
> > probably due to historical reasons.  Further, at least some of these
> > functions no longer perform any allocation; they are just
> > initialization functions now that allocation has been taken to the LSM
> > framework, so possibly could be renamed and made to return void at some point.
> 
> I've noticed the same thing on a few occasions, I've just never bothered to put
> the fixes into a patch.  We might as well do that now, at least for the redundant
> code bits; I'll leave the return code issue for another time as that would cross
> LSM boundaries and that really isn't appropriate in the -rc5 timeframe IMHO.
> 
> I'll put something together once I finish up the patch/review backlog from the
> past few days.  Looking quickly with a regex, it would appear that
> inode_alloc_security(), file_alloc_security(), and
> superblock_alloc_security() are all candidates.  While not an allocator, we can
> probably get rid of inode_doinit() as well.

Besides, it looks like selinux_nlmsg_perm is candidate too.
Just send a patch for this function.

Cheers,
Huaisheng Ye

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] selinux: remove redundant msg_msg_alloc_security
@ 2020-01-10 16:46 Huaisheng HS1 Ye
  0 siblings, 0 replies; 7+ messages in thread
From: Huaisheng HS1 Ye @ 2020-01-10 16:46 UTC (permalink / raw)
  To: Stephen Smalley, paul, eparis, jmorris, serge
  Cc: Tzu ting Yu1, linux-security-module, selinux, linux-kernel, Huaisheng Ye


> -----Original Message-----
> From: Stephen Smalley <sds@tycho.nsa.gov>
> Sent: Friday, January 10, 2020 11:14 PM
> 
> On 1/10/20 4:58 AM, Huaisheng Ye wrote:
> > From: Huaisheng Ye <yehs1@lenovo.com>
> >
> > selinux_msg_msg_alloc_security only calls msg_msg_alloc_security but
> > do nothing else. And also msg_msg_alloc_security is just used by the
> > former.
> >
> > Remove the redundant function to simplify the code.
> 
> This seems to also be true of other _alloc_security functions, probably due to
> historical reasons.  Further, at least some of these functions no longer perform
> any allocation; they are just initialization functions now that allocation has
> been taken to the LSM framework, so possibly could be renamed and made to return
> void at some point.
> 
> >
> > Signed-off-by: Huaisheng Ye <yehs1@lenovo.com>
> 
> Acked-by: Stephen Smalley <sds@tycho.nsa.gov>

Many thanks for the Acked-by.

Yes, you are right, selinux_msg_msg_alloc_security only can return 0 in any case.
I think that should be modified to void instead of int.

And also, the fact is no module needs to implement msg_msg_free_security, because
LSM would take the responsibility for freeing msg->security.
I think we could delete the hook call of msg_msg_free_security, but I am cautious
to modify the existing interfaces, that just worry to break traditional rules.

Cheers,
Huaisheng Ye
Lenovo


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-01-12 15:45 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-10  9:58 [PATCH] selinux: remove redundant msg_msg_alloc_security Huaisheng Ye
2020-01-10 15:13 ` Stephen Smalley
2020-01-10 16:44   ` Casey Schaufler
2020-01-10 16:50   ` Paul Moore
2020-01-12 15:45     ` [External] " Huaisheng HS1 Ye
2020-01-10 16:43 ` Paul Moore
2020-01-10 16:46 Huaisheng HS1 Ye

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.