diff mbox series

[RFC,1/3] selinux: refactor sidtab conversion

Message ID 20181113135255.26045-2-omosnace@redhat.com (mailing list archive)
State Accepted
Headers show
Series Fix ENOMEM errors during policy reload | expand

Commit Message

Ondrej Mosnacek Nov. 13, 2018, 1:52 p.m. UTC 
This is a purely cosmetic change that encapsulates the three-step sidtab
conversion logic (shutdown -> clone -> map) into a single function
defined in sidtab.c (as opposed to services.c).

Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
---
 security/selinux/ss/services.c | 22 +--------------
 security/selinux/ss/sidtab.c   | 50 ++++++++++++++++++++++++----------
 security/selinux/ss/sidtab.h   | 11 ++++----
 3 files changed, 42 insertions(+), 41 deletions(-)

Comments

Stephen Smalley Nov. 13, 2018, 9:19 p.m. UTC | #1 
On 11/13/18 8:52 AM, Ondrej Mosnacek wrote:
> This is a purely cosmetic change that encapsulates the three-step sidtab
> conversion logic (shutdown -> clone -> map) into a single function
> defined in sidtab.c (as opposed to services.c).
> 
> Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>

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

> ---
>   security/selinux/ss/services.c | 22 +--------------
>   security/selinux/ss/sidtab.c   | 50 ++++++++++++++++++++++++----------
>   security/selinux/ss/sidtab.h   | 11 ++++----
>   3 files changed, 42 insertions(+), 41 deletions(-)
> 
> diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
> index 12e414394530..7337db24a6a8 100644
> --- a/security/selinux/ss/services.c
> +++ b/security/selinux/ss/services.c
> @@ -1880,19 +1880,6 @@ int security_change_sid(struct selinux_state *state,
>   				    out_sid, false);
>   }
>   
> -/* Clone the SID into the new SID table. */
> -static int clone_sid(u32 sid,
> -		     struct context *context,
> -		     void *arg)
> -{
> -	struct sidtab *s = arg;
> -
> -	if (sid > SECINITSID_NUM)
> -		return sidtab_insert(s, sid, context);
> -	else
> -		return 0;
> -}
> -
>   static inline int convert_context_handle_invalid_context(
>   	struct selinux_state *state,
>   	struct context *context)
> @@ -2186,13 +2173,6 @@ int security_load_policy(struct selinux_state *state, void *data, size_t len)
>   		goto err;
>   	}
>   
> -	/* Clone the SID table. */
> -	sidtab_shutdown(sidtab);
> -
> -	rc = sidtab_map(sidtab, clone_sid, &newsidtab);
> -	if (rc)
> -		goto err;
> -
>   	/*
>   	 * Convert the internal representations of contexts
>   	 * in the new SID table.
> @@ -2200,7 +2180,7 @@ int security_load_policy(struct selinux_state *state, void *data, size_t len)
>   	args.state = state;
>   	args.oldp = policydb;
>   	args.newp = newpolicydb;
> -	rc = sidtab_map(&newsidtab, convert_context, &args);
> +	rc = sidtab_convert(sidtab, &newsidtab, convert_context, &args);
>   	if (rc) {
>   		pr_err("SELinux:  unable to convert the internal"
>   			" representation of contexts in the new SID"
> diff --git a/security/selinux/ss/sidtab.c b/security/selinux/ss/sidtab.c
> index fd75a12fa8fc..e66a2ab3d1c2 100644
> --- a/security/selinux/ss/sidtab.c
> +++ b/security/selinux/ss/sidtab.c
> @@ -116,11 +116,11 @@ struct context *sidtab_search_force(struct sidtab *s, u32 sid)
>   	return sidtab_search_core(s, sid, 1);
>   }
>   
> -int sidtab_map(struct sidtab *s,
> -	       int (*apply) (u32 sid,
> -			     struct context *context,
> -			     void *args),
> -	       void *args)
> +static int sidtab_map(struct sidtab *s,
> +		      int (*apply) (u32 sid,
> +				    struct context *context,
> +				    void *args),
> +		      void *args)
>   {
>   	int i, rc = 0;
>   	struct sidtab_node *cur;
> @@ -141,6 +141,37 @@ out:
>   	return rc;
>   }
>   
> +/* Clone the SID into the new SID table. */
> +static int clone_sid(u32 sid, struct context *context, void *arg)
> +{
> +	struct sidtab *s = arg;
> +
> +	if (sid > SECINITSID_NUM)
> +		return sidtab_insert(s, sid, context);
> +	else
> +		return 0;
> +}
> +
> +int sidtab_convert(struct sidtab *s, struct sidtab *news,
> +		   int (*convert) (u32 sid,
> +				   struct context *context,
> +				   void *args),
> +		   void *args)
> +{
> +	unsigned long flags;
> +	int rc;
> +
> +	spin_lock_irqsave(&s->lock, flags);
> +	s->shutdown = 1;
> +	spin_unlock_irqrestore(&s->lock, flags);
> +
> +	rc = sidtab_map(s, clone_sid, news);
> +	if (rc)
> +		return rc;
> +
> +	return sidtab_map(news, convert, args);
> +}
> +
>   static void sidtab_update_cache(struct sidtab *s, struct sidtab_node *n, int loc)
>   {
>   	BUG_ON(loc >= SIDTAB_CACHE_LEN);
> @@ -295,12 +326,3 @@ void sidtab_set(struct sidtab *dst, struct sidtab *src)
>   		dst->cache[i] = NULL;
>   	spin_unlock_irqrestore(&src->lock, flags);
>   }
> -
> -void sidtab_shutdown(struct sidtab *s)
> -{
> -	unsigned long flags;
> -
> -	spin_lock_irqsave(&s->lock, flags);
> -	s->shutdown = 1;
> -	spin_unlock_irqrestore(&s->lock, flags);
> -}
> diff --git a/security/selinux/ss/sidtab.h b/security/selinux/ss/sidtab.h
> index a1a1d2617b6f..26c74fe7afc0 100644
> --- a/security/selinux/ss/sidtab.h
> +++ b/security/selinux/ss/sidtab.h
> @@ -37,11 +37,11 @@ int sidtab_insert(struct sidtab *s, u32 sid, struct context *context);
>   struct context *sidtab_search(struct sidtab *s, u32 sid);
>   struct context *sidtab_search_force(struct sidtab *s, u32 sid);
>   
> -int sidtab_map(struct sidtab *s,
> -	       int (*apply) (u32 sid,
> -			     struct context *context,
> -			     void *args),
> -	       void *args);
> +int sidtab_convert(struct sidtab *s, struct sidtab *news,
> +		   int (*apply) (u32 sid,
> +				 struct context *context,
> +				 void *args),
> +		   void *args);
>   
>   int sidtab_context_to_sid(struct sidtab *s,
>   			  struct context *context,
> @@ -50,7 +50,6 @@ int sidtab_context_to_sid(struct sidtab *s,
>   void sidtab_hash_eval(struct sidtab *h, char *tag);
>   void sidtab_destroy(struct sidtab *s);
>   void sidtab_set(struct sidtab *dst, struct sidtab *src);
> -void sidtab_shutdown(struct sidtab *s);
>   
>   #endif	/* _SS_SIDTAB_H_ */
>   
>
Paul Moore Nov. 20, 2018, 9:47 p.m. UTC | #2 
On Tue, Nov 13, 2018 at 8:53 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> This is a purely cosmetic change that encapsulates the three-step sidtab
> conversion logic (shutdown -> clone -> map) into a single function
> defined in sidtab.c (as opposed to services.c).
>
> Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
> ---
>  security/selinux/ss/services.c | 22 +--------------
>  security/selinux/ss/sidtab.c   | 50 ++++++++++++++++++++++++----------
>  security/selinux/ss/sidtab.h   | 11 ++++----
>  3 files changed, 42 insertions(+), 41 deletions(-)

Merged into selinux/next with some whitespace fixes (inherited from
code you cut n' pasted).  Please remember to run your patches through
scripts/checkpatch.pl before submission.
Ondrej Mosnacek Nov. 21, 2018, 8:08 a.m. UTC | #3 
On Tue, Nov 20, 2018 at 10:47 PM Paul Moore <paul@paul-moore.com> wrote:
> On Tue, Nov 13, 2018 at 8:53 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> > This is a purely cosmetic change that encapsulates the three-step sidtab
> > conversion logic (shutdown -> clone -> map) into a single function
> > defined in sidtab.c (as opposed to services.c).
> >
> > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
> > ---
> >  security/selinux/ss/services.c | 22 +--------------
> >  security/selinux/ss/sidtab.c   | 50 ++++++++++++++++++++++++----------
> >  security/selinux/ss/sidtab.h   | 11 ++++----
> >  3 files changed, 42 insertions(+), 41 deletions(-)
>
> Merged into selinux/next with some whitespace fixes (inherited from
> code you cut n' pasted).  Please remember to run your patches through
> scripts/checkpatch.pl before submission.

Damn, I still haven't set up that commit hook... Done now, seems to
work fine. Sorry for not having done that sooner and thanks for
nagging me about it :)
diff mbox series

Patch

diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index 12e414394530..7337db24a6a8 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -1880,19 +1880,6 @@  int security_change_sid(struct selinux_state *state,
 				    out_sid, false);
 }
 
-/* Clone the SID into the new SID table. */
-static int clone_sid(u32 sid,
-		     struct context *context,
-		     void *arg)
-{
-	struct sidtab *s = arg;
-
-	if (sid > SECINITSID_NUM)
-		return sidtab_insert(s, sid, context);
-	else
-		return 0;
-}
-
 static inline int convert_context_handle_invalid_context(
 	struct selinux_state *state,
 	struct context *context)
@@ -2186,13 +2173,6 @@  int security_load_policy(struct selinux_state *state, void *data, size_t len)
 		goto err;
 	}
 
-	/* Clone the SID table. */
-	sidtab_shutdown(sidtab);
-
-	rc = sidtab_map(sidtab, clone_sid, &newsidtab);
-	if (rc)
-		goto err;
-
 	/*
 	 * Convert the internal representations of contexts
 	 * in the new SID table.
@@ -2200,7 +2180,7 @@  int security_load_policy(struct selinux_state *state, void *data, size_t len)
 	args.state = state;
 	args.oldp = policydb;
 	args.newp = newpolicydb;
-	rc = sidtab_map(&newsidtab, convert_context, &args);
+	rc = sidtab_convert(sidtab, &newsidtab, convert_context, &args);
 	if (rc) {
 		pr_err("SELinux:  unable to convert the internal"
 			" representation of contexts in the new SID"
diff --git a/security/selinux/ss/sidtab.c b/security/selinux/ss/sidtab.c
index fd75a12fa8fc..e66a2ab3d1c2 100644
--- a/security/selinux/ss/sidtab.c
+++ b/security/selinux/ss/sidtab.c
@@ -116,11 +116,11 @@  struct context *sidtab_search_force(struct sidtab *s, u32 sid)
 	return sidtab_search_core(s, sid, 1);
 }
 
-int sidtab_map(struct sidtab *s,
-	       int (*apply) (u32 sid,
-			     struct context *context,
-			     void *args),
-	       void *args)
+static int sidtab_map(struct sidtab *s,
+		      int (*apply) (u32 sid,
+				    struct context *context,
+				    void *args),
+		      void *args)
 {
 	int i, rc = 0;
 	struct sidtab_node *cur;
@@ -141,6 +141,37 @@  out:
 	return rc;
 }
 
+/* Clone the SID into the new SID table. */
+static int clone_sid(u32 sid, struct context *context, void *arg)
+{
+	struct sidtab *s = arg;
+
+	if (sid > SECINITSID_NUM)
+		return sidtab_insert(s, sid, context);
+	else
+		return 0;
+}
+
+int sidtab_convert(struct sidtab *s, struct sidtab *news,
+		   int (*convert) (u32 sid,
+				   struct context *context,
+				   void *args),
+		   void *args)
+{
+	unsigned long flags;
+	int rc;
+
+	spin_lock_irqsave(&s->lock, flags);
+	s->shutdown = 1;
+	spin_unlock_irqrestore(&s->lock, flags);
+
+	rc = sidtab_map(s, clone_sid, news);
+	if (rc)
+		return rc;
+
+	return sidtab_map(news, convert, args);
+}
+
 static void sidtab_update_cache(struct sidtab *s, struct sidtab_node *n, int loc)
 {
 	BUG_ON(loc >= SIDTAB_CACHE_LEN);
@@ -295,12 +326,3 @@  void sidtab_set(struct sidtab *dst, struct sidtab *src)
 		dst->cache[i] = NULL;
 	spin_unlock_irqrestore(&src->lock, flags);
 }
-
-void sidtab_shutdown(struct sidtab *s)
-{
-	unsigned long flags;
-
-	spin_lock_irqsave(&s->lock, flags);
-	s->shutdown = 1;
-	spin_unlock_irqrestore(&s->lock, flags);
-}
diff --git a/security/selinux/ss/sidtab.h b/security/selinux/ss/sidtab.h
index a1a1d2617b6f..26c74fe7afc0 100644
--- a/security/selinux/ss/sidtab.h
+++ b/security/selinux/ss/sidtab.h
@@ -37,11 +37,11 @@  int sidtab_insert(struct sidtab *s, u32 sid, struct context *context);
 struct context *sidtab_search(struct sidtab *s, u32 sid);
 struct context *sidtab_search_force(struct sidtab *s, u32 sid);
 
-int sidtab_map(struct sidtab *s,
-	       int (*apply) (u32 sid,
-			     struct context *context,
-			     void *args),
-	       void *args);
+int sidtab_convert(struct sidtab *s, struct sidtab *news,
+		   int (*apply) (u32 sid,
+				 struct context *context,
+				 void *args),
+		   void *args);
 
 int sidtab_context_to_sid(struct sidtab *s,
 			  struct context *context,
@@ -50,7 +50,6 @@  int sidtab_context_to_sid(struct sidtab *s,
 void sidtab_hash_eval(struct sidtab *h, char *tag);
 void sidtab_destroy(struct sidtab *s);
 void sidtab_set(struct sidtab *dst, struct sidtab *src);
-void sidtab_shutdown(struct sidtab *s);
 
 #endif	/* _SS_SIDTAB_H_ */