Author: KaiGai Kohei

Email: kaigai@ak.jp.nec.com
Subject: libselinux APIs should take "const" qualifier?
Date: Tue, 23 Mar 2010 11:56:36 +0900

(2010/03/19 22:32), Stephen Smalley wrote:
> On Fri, 2010-03-19 at 16:52 +0900, KaiGai Kohei wrote:
>> Right now, security_context_t is an alias of char *, declared in selinux.h.
>>
>> Various kind of libselinux API takes security_context_t arguments,
>> however, it is inconvenience in several situations.
>>
>> For example, the following query is parsed, then delivered to access
>> control subsystem with the security context as "const char *" cstring.
>>
>>    ALTER TABLE my_tbl SECURITY LABEL TO 'system_u:object_r:sepgsql_table_t:SystemHigh';
>>                  const char *<----    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>
>> In this case, we want to call selinux_trans_to_raw_context() to translate
>> the given security context into raw format. But it takes security_context_t
>> argument for the source context, although this pointer is read-only.
>> In the result, compiler raises warnings because we gave "const char *" pointer
>> into functions which take security_context_t (= char *).
>>
>> Any comments?
>>
>> It seems to me the following functions' prototype should be qualified by
>> "const".
>
> That seems reasonable and should have no impact on library ABI.
> On the other hand, others have pointed out that security_context_t is
> not a properly encapsulated data type at all, and perhaps should be
> deprecated and replaced with direct use of char*/const char* throughout.
>
> There are other library API issues as well that have come up in the
> past, such as lack of adequate namespacing (with approaches put forth),
> but we don't ever seem to get a round tuit.

At first, I tried to add const qualifiers read-only security_context_t
pointers, but didn't replace them by char */const char * yet, right now.

BTW, I could find out the following code:

  int security_compute_create(security_context_t scon,
                              security_context_t tcon,
                              security_class_t tclass,
                              security_context_t * newcon)
  {
          int ret;
          security_context_t rscon = scon;
          security_context_t rtcon = tcon;
          security_context_t rnewcon;

          if (selinux_trans_to_raw_context(scon, &rscon))
                  return -1;
          if (selinux_trans_to_raw_context(tcon, &rtcon)) {
                  freecon(rscon);
                  return -1;
          }
      :

In this case, scon and tcon can be qualified by const, and the first
argument of selinux_trans_to_raw_context() can take const pointer.
But it tries to initialize rscon and tscon by const pointer, although
these are used to store raw security contexts.
The selinux_trans_to_raw_context() always set dynamically allocated
text string on the second argument, so we don't need to initialize it
anyway. I also removed these initializations in this patch.

Does the older mcstrans code could return without allocation of raw
format when the given scon is already raw format? I don't know why
these are initialized in this manner.

Thanks.
--
KaiGai Kohei <kaigai@ak.jp.nec.com>

Signed-off-by: Chad Sellers <csellers@tresys.com>

16 files changed, 64 insertions, 64 deletions

diff --git a/libselinux/src/avc.c b/libselinux/src/avc.c
index e9e35296..74591b4f 100644
--- a/libselinux/src/avc.c
+++ b/libselinux/src/avc.c
@@ -66,7 +66,7 @@ static inline int avc_hash(security_id_t ssid,
 	    & (AVC_CACHE_SLOTS - 1);
 }
 
-int avc_context_to_sid_raw(security_context_t ctx, security_id_t * sid)
+int avc_context_to_sid_raw(const security_context_t ctx, security_id_t * sid)
 {
 	int rc;
 	avc_get_lock(avc_lock);
@@ -75,7 +75,7 @@ int avc_context_to_sid_raw(security_context_t ctx, security_id_t * sid)
 	return rc;
 }
 
-int avc_context_to_sid(security_context_t ctx, security_id_t * sid)
+int avc_context_to_sid(const security_context_t ctx, security_id_t * sid)
 {
 	int ret;
 	security_context_t rctx;
diff --git a/libselinux/src/avc_sidtab.c b/libselinux/src/avc_sidtab.c
index 3ca1d1fd..0b696bb8 100644
--- a/libselinux/src/avc_sidtab.c
+++ b/libselinux/src/avc_sidtab.c
@@ -46,7 +46,7 @@ int sidtab_init(struct sidtab *s)
 	return rc;
 }
 
-int sidtab_insert(struct sidtab *s, security_context_t ctx)
+int sidtab_insert(struct sidtab *s, const security_context_t ctx)
 {
 	int hvalue, rc = 0;
 	struct sidtab_node *newnode;
@@ -76,7 +76,7 @@ int sidtab_insert(struct sidtab *s, security_context_t ctx)
 
 int
 sidtab_context_to_sid(struct sidtab *s,
-		      security_context_t ctx, security_id_t * sid)
+		      const security_context_t ctx, security_id_t * sid)
 {
 	int hvalue, rc = 0;
 	struct sidtab_node *cur;
diff --git a/libselinux/src/canonicalize_context.c b/libselinux/src/canonicalize_context.c
index 85bbbfad..176c45aa 100644
--- a/libselinux/src/canonicalize_context.c
+++ b/libselinux/src/canonicalize_context.c
@@ -9,7 +9,7 @@
 #include "policy.h"
 #include <limits.h>
 
-int security_canonicalize_context_raw(security_context_t con,
+int security_canonicalize_context_raw(const security_context_t con,
 				      security_context_t * canoncon)
 {
 	char path[PATH_MAX];
@@ -62,11 +62,11 @@ int security_canonicalize_context_raw(security_context_t con,
 
 hidden_def(security_canonicalize_context_raw)
 
-int security_canonicalize_context(security_context_t con,
+int security_canonicalize_context(const security_context_t con,
 				  security_context_t * canoncon)
 {
 	int ret;
-	security_context_t rcon = con;
+	security_context_t rcon;
 	security_context_t rcanoncon;
 
 	if (selinux_trans_to_raw_context(con, &rcon))
diff --git a/libselinux/src/check_context.c b/libselinux/src/check_context.c
index 0e8fb864..33ab5e31 100644
--- a/libselinux/src/check_context.c
+++ b/libselinux/src/check_context.c
@@ -9,7 +9,7 @@
 #include "policy.h"
 #include <limits.h>
 
-int security_check_context_raw(security_context_t con)
+int security_check_context_raw(const security_context_t con)
 {
 	char path[PATH_MAX];
 	int fd, ret;
@@ -33,10 +33,10 @@ int security_check_context_raw(security_context_t con)
 
 hidden_def(security_check_context_raw)
 
-int security_check_context(security_context_t con)
+int security_check_context(const security_context_t con)
 {
 	int ret;
-	security_context_t rcon = con;
+	security_context_t rcon;
 
 	if (selinux_trans_to_raw_context(con, &rcon))
 		return -1;
diff --git a/libselinux/src/compute_av.c b/libselinux/src/compute_av.c
index a821d178..86ff2740 100644
--- a/libselinux/src/compute_av.c
+++ b/libselinux/src/compute_av.c
@@ -10,8 +10,8 @@
 #include "policy.h"
 #include "mapping.h"
 
-int security_compute_av_flags_raw(security_context_t scon,
-				  security_context_t tcon,
+int security_compute_av_flags_raw(const security_context_t scon,
+				  const security_context_t tcon,
 				  security_class_t tclass,
 				  access_vector_t requested,
 				  struct av_decision *avd)
@@ -72,8 +72,8 @@ int security_compute_av_flags_raw(security_context_t scon,
 
 hidden_def(security_compute_av_flags_raw)
 
-int security_compute_av_raw(security_context_t scon,
-			    security_context_t tcon,
+int security_compute_av_raw(const security_context_t scon,
+			    const security_context_t tcon,
 			    security_class_t tclass,
 			    access_vector_t requested,
 			    struct av_decision *avd)
@@ -99,14 +99,14 @@ int security_compute_av_raw(security_context_t scon,
 
 hidden_def(security_compute_av_raw)
 
-int security_compute_av_flags(security_context_t scon,
-			      security_context_t tcon,
+int security_compute_av_flags(const security_context_t scon,
+			      const security_context_t tcon,
 			      security_class_t tclass,
 			      access_vector_t requested,
 			      struct av_decision *avd)
 {
-	security_context_t rscon = scon;
-	security_context_t rtcon = tcon;
+	security_context_t rscon;
+	security_context_t rtcon;
 	int ret;
 
 	if (selinux_trans_to_raw_context(scon, &rscon))
@@ -126,8 +126,8 @@ int security_compute_av_flags(security_context_t scon,
 
 hidden_def(security_compute_av_flags)
 
-int security_compute_av(security_context_t scon,
-			security_context_t tcon,
+int security_compute_av(const security_context_t scon,
+			const security_context_t tcon,
 			security_class_t tclass,
 			access_vector_t requested, struct av_decision *avd)
 {
diff --git a/libselinux/src/compute_create.c b/libselinux/src/compute_create.c
index 1c56f0fb..0bbeeed6 100644
--- a/libselinux/src/compute_create.c
+++ b/libselinux/src/compute_create.c
@@ -10,8 +10,8 @@
 #include "policy.h"
 #include "mapping.h"
 
-int security_compute_create_raw(security_context_t scon,
-				security_context_t tcon,
+int security_compute_create_raw(const security_context_t scon,
+				const security_context_t tcon,
 				security_class_t tclass,
 				security_context_t * newcon)
 {
@@ -62,14 +62,14 @@ int security_compute_create_raw(security_context_t scon,
 
 hidden_def(security_compute_create_raw)
 
-int security_compute_create(security_context_t scon,
-			    security_context_t tcon,
+int security_compute_create(const security_context_t scon,
+			    const security_context_t tcon,
 			    security_class_t tclass,
 			    security_context_t * newcon)
 {
 	int ret;
-	security_context_t rscon = scon;
-	security_context_t rtcon = tcon;
+	security_context_t rscon;
+	security_context_t rtcon;
 	security_context_t rnewcon;
 
 	if (selinux_trans_to_raw_context(scon, &rscon))
diff --git a/libselinux/src/compute_member.c b/libselinux/src/compute_member.c
index e5495e44..dad0a775 100644
--- a/libselinux/src/compute_member.c
+++ b/libselinux/src/compute_member.c
@@ -10,8 +10,8 @@
 #include "policy.h"
 #include "mapping.h"
 
-int security_compute_member_raw(security_context_t scon,
-				security_context_t tcon,
+int security_compute_member_raw(const security_context_t scon,
+				const security_context_t tcon,
 				security_class_t tclass,
 				security_context_t * newcon)
 {
@@ -62,14 +62,14 @@ int security_compute_member_raw(security_context_t scon,
 
 hidden_def(security_compute_member_raw)
 
-int security_compute_member(security_context_t scon,
-			    security_context_t tcon,
+int security_compute_member(const security_context_t scon,
+			    const security_context_t tcon,
 			    security_class_t tclass,
 			    security_context_t * newcon)
 {
 	int ret;
-	security_context_t rscon = scon;
-	security_context_t rtcon = tcon;
+	security_context_t rscon;
+	security_context_t rtcon;
 	security_context_t rnewcon;
 
 	if (selinux_trans_to_raw_context(scon, &rscon))
diff --git a/libselinux/src/compute_relabel.c b/libselinux/src/compute_relabel.c
index ae9c6483..656f00af 100644
--- a/libselinux/src/compute_relabel.c
+++ b/libselinux/src/compute_relabel.c
@@ -10,8 +10,8 @@
 #include "policy.h"
 #include "mapping.h"
 
-int security_compute_relabel_raw(security_context_t scon,
-				 security_context_t tcon,
+int security_compute_relabel_raw(const security_context_t scon,
+				 const security_context_t tcon,
 				 security_class_t tclass,
 				 security_context_t * newcon)
 {
@@ -62,14 +62,14 @@ int security_compute_relabel_raw(security_context_t scon,
 
 hidden_def(security_compute_relabel_raw)
 
-int security_compute_relabel(security_context_t scon,
-			     security_context_t tcon,
+int security_compute_relabel(const security_context_t scon,
+			     const security_context_t tcon,
 			     security_class_t tclass,
 			     security_context_t * newcon)
 {
 	int ret;
-	security_context_t rscon = scon;
-	security_context_t rtcon = tcon;
+	security_context_t rscon;
+	security_context_t rtcon;
 	security_context_t rnewcon;
 
 	if (selinux_trans_to_raw_context(scon, &rscon))
diff --git a/libselinux/src/compute_user.c b/libselinux/src/compute_user.c
index fa6f650a..3b39ddd1 100644
--- a/libselinux/src/compute_user.c
+++ b/libselinux/src/compute_user.c
@@ -9,7 +9,7 @@
 #include "policy.h"
 #include <limits.h>
 
-int security_compute_user_raw(security_context_t scon,
+int security_compute_user_raw(const security_context_t scon,
 			      const char *user, security_context_t ** con)
 {
 	char path[PATH_MAX];
@@ -79,11 +79,11 @@ int security_compute_user_raw(security_context_t scon,
 
 hidden_def(security_compute_user_raw)
 
-int security_compute_user(security_context_t scon,
+int security_compute_user(const security_context_t scon,
 			  const char *user, security_context_t ** con)
 {
 	int ret;
-	security_context_t rscon = scon;
+	security_context_t rscon;
 
 	if (selinux_trans_to_raw_context(scon, &rscon))
 		return -1;
diff --git a/libselinux/src/fsetfilecon.c b/libselinux/src/fsetfilecon.c
index 6cad4d7e..309105c2 100644
--- a/libselinux/src/fsetfilecon.c
+++ b/libselinux/src/fsetfilecon.c
@@ -7,7 +7,7 @@
 #include "selinux_internal.h"
 #include "policy.h"
 
-int fsetfilecon_raw(int fd, security_context_t context)
+int fsetfilecon_raw(int fd, const security_context_t context)
 {
 	return fsetxattr(fd, XATTR_NAME_SELINUX, context, strlen(context) + 1,
 			 0);
@@ -15,10 +15,10 @@ int fsetfilecon_raw(int fd, security_context_t context)
 
 hidden_def(fsetfilecon_raw)
 
-int fsetfilecon(int fd, security_context_t context)
+int fsetfilecon(int fd, const security_context_t context)
 {
 	int ret;
-	security_context_t rcontext = context;
+	security_context_t rcontext;
 
 	if (selinux_trans_to_raw_context(context, &rcontext))
 		return -1;
diff --git a/libselinux/src/is_customizable_type.c b/libselinux/src/is_customizable_type.c
index 67854050..a8e2183e 100644
--- a/libselinux/src/is_customizable_type.c
+++ b/libselinux/src/is_customizable_type.c
@@ -61,7 +61,7 @@ static int get_customizable_type_list(security_context_t ** retlist)
 
 static security_context_t *customizable_list = NULL;
 
-int is_context_customizable(security_context_t scontext)
+int is_context_customizable(const security_context_t scontext)
 {
 	int i;
 	const char *type;
diff --git a/libselinux/src/lsetfilecon.c b/libselinux/src/lsetfilecon.c
index 844e5c7d..461e3f7c 100644
--- a/libselinux/src/lsetfilecon.c
+++ b/libselinux/src/lsetfilecon.c
@@ -7,7 +7,7 @@
 #include "selinux_internal.h"
 #include "policy.h"
 
-int lsetfilecon_raw(const char *path, security_context_t context)
+int lsetfilecon_raw(const char *path, const security_context_t context)
 {
 	return lsetxattr(path, XATTR_NAME_SELINUX, context, strlen(context) + 1,
 			 0);
@@ -15,10 +15,10 @@ int lsetfilecon_raw(const char *path, security_context_t context)
 
 hidden_def(lsetfilecon_raw)
 
-int lsetfilecon(const char *path, security_context_t context)
+int lsetfilecon(const char *path, const security_context_t context)
 {
 	int ret;
-	security_context_t rcontext = context;
+	security_context_t rcontext;
 
 	if (selinux_trans_to_raw_context(context, &rcontext))
 		return -1;
diff --git a/libselinux/src/procattr.c b/libselinux/src/procattr.c
index 8f3f4014..83381e4b 100644
--- a/libselinux/src/procattr.c
+++ b/libselinux/src/procattr.c
@@ -126,11 +126,11 @@ static int setprocattrcon_raw(security_context_t context,
 		return 0;
 }
 
-static int setprocattrcon(security_context_t context,
+static int setprocattrcon(const security_context_t context,
 			  pid_t pid, const char *attr)
 {
 	int ret;
-	security_context_t rcontext = context;
+	security_context_t rcontext;
 
 	if (selinux_trans_to_raw_context(context, &rcontext))
 		return -1;
@@ -153,11 +153,11 @@ static int setprocattrcon(security_context_t context,
 	}
 
 #define setselfattr_def(fn, attr) \
-	int set##fn##_raw(security_context_t c) \
+	int set##fn##_raw(const security_context_t c) \
 	{ \
 		return setprocattrcon_raw(c, 0, #attr); \
 	} \
-	int set##fn(security_context_t c) \
+	int set##fn(const security_context_t c) \
 	{ \
 		return setprocattrcon(c, 0, #attr); \
 	}
diff --git a/libselinux/src/selinux_check_securetty_context.c b/libselinux/src/selinux_check_securetty_context.c
index fdb5a592..e6d25017 100644
--- a/libselinux/src/selinux_check_securetty_context.c
+++ b/libselinux/src/selinux_check_securetty_context.c
@@ -6,7 +6,7 @@
 #include "selinux_internal.h"
 #include "context_internal.h"
 
-int selinux_check_securetty_context(security_context_t tty_context)
+int selinux_check_securetty_context(const security_context_t tty_context)
 {
 	char *line = NULL;
 	char *start, *end = NULL;
diff --git a/libselinux/src/setfilecon.c b/libselinux/src/setfilecon.c
index 8c633efb..7465c6a4 100644
--- a/libselinux/src/setfilecon.c
+++ b/libselinux/src/setfilecon.c
@@ -7,7 +7,7 @@
 #include "selinux_internal.h"
 #include "policy.h"
 
-int setfilecon_raw(const char *path, security_context_t context)
+int setfilecon_raw(const char *path, const security_context_t context)
 {
 	return setxattr(path, XATTR_NAME_SELINUX, context, strlen(context) + 1,
 			0);
@@ -15,10 +15,10 @@ int setfilecon_raw(const char *path, security_context_t context)
 
 hidden_def(setfilecon_raw)
 
-int setfilecon(const char *path, security_context_t context)
+int setfilecon(const char *path, const security_context_t context)
 {
 	int ret;
-	security_context_t rcontext = context;
+	security_context_t rcontext;
 
 	if (selinux_trans_to_raw_context(context, &rcontext))
 		return -1;
diff --git a/libselinux/src/setrans_client.c b/libselinux/src/setrans_client.c
index 2bbcbde1..ef54442e 100644
--- a/libselinux/src/setrans_client.c
+++ b/libselinux/src/setrans_client.c
@@ -165,7 +165,7 @@ receive_response(int fd, uint32_t function, char **outdata, int32_t * ret_val)
 	return 0;
 }
 
-static int raw_to_trans_context(char *raw, char **transp)
+static int raw_to_trans_context(const char *raw, char **transp)
 {
 	int ret;
 	int32_t ret_val;
@@ -191,7 +191,7 @@ static int raw_to_trans_context(char *raw, char **transp)
 	return ret;
 }
 
-static int trans_to_raw_context(char *trans, char **rawp)
+static int trans_to_raw_context(const char *trans, char **rawp)
 {
 	int ret;
 	int32_t ret_val;
@@ -216,7 +216,7 @@ static int trans_to_raw_context(char *trans, char **rawp)
 	return ret;
 }
 
-static int raw_context_to_color(char *raw, char **colors)
+static int raw_context_to_color(const char *raw, char **colors)
 {
 	int ret;
 	int32_t ret_val;
@@ -245,7 +245,7 @@ static void init_context_translations(void)
 	mls_enabled = is_selinux_mls_enabled();
 }
 
-int selinux_trans_to_raw_context(security_context_t trans,
+int selinux_trans_to_raw_context(const security_context_t trans,
 				 security_context_t * rawp)
 {
 	if (!trans) {
@@ -286,7 +286,7 @@ int selinux_trans_to_raw_context(security_context_t trans,
 
 hidden_def(selinux_trans_to_raw_context)
 
-int selinux_raw_to_trans_context(security_context_t raw,
+int selinux_raw_to_trans_context(const security_context_t raw,
 				 security_context_t * transp)
 {
 	if (!raw) {
@@ -327,7 +327,7 @@ int selinux_raw_to_trans_context(security_context_t raw,
 
 hidden_def(selinux_raw_to_trans_context)
 
-int selinux_raw_context_to_color(security_context_t raw, char **transp)
+int selinux_raw_context_to_color(const security_context_t raw, char **transp)
 {
 	if (!raw) {
 		*transp = NULL;
@@ -361,7 +361,7 @@ int selinux_raw_context_to_color(security_context_t raw, char **transp)
 hidden_def(selinux_raw_context_to_color)
 #else /*DISABLE_SETRANS*/
 
-int selinux_trans_to_raw_context(security_context_t trans,
+int selinux_trans_to_raw_context(const security_context_t trans,
 				 security_context_t * rawp)
 {
 	if (!trans) {
@@ -376,7 +376,7 @@ int selinux_trans_to_raw_context(security_context_t trans,
 
 hidden_def(selinux_trans_to_raw_context)
 
-int selinux_raw_to_trans_context(security_context_t raw,
+int selinux_raw_to_trans_context(const security_context_t raw,
 				 security_context_t * transp)
 {
 	if (!raw) {