On 3/22/10 10:56 PM, "KaiGai Kohei" <kaigai@xxxxxxxxxxxxx> wrote: > (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. Acked-by: Chad Sellers <csellers@xxxxxxxxxx> Merged as of libselinux 2.0.96 -- This message was distributed to subscribers of the selinux mailing list. If you no longer wish to subscribe, send mail to majordomo@xxxxxxxxxxxxx with the words "unsubscribe selinux" without quotes as the message.