Re: [RFC PATCH 1/3] selinux: refactor sidtab conversion

[Stephen Smalley]

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 <omosn...@redhat.com>

Acked-by: Stephen Smalley <s...@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_ */

_______________________________________________
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.