* libselinux APIs should take "const" qualifier?
@ 2010-03-19 7:52 KaiGai Kohei
2010-03-19 13:32 ` Stephen Smalley
0 siblings, 1 reply; 6+ messages in thread
From: KaiGai Kohei @ 2010-03-19 7:52 UTC (permalink / raw)
To: SELinux
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".
extern int setcon(*const* security_context_t con);
extern int setexeccon(*const* security_context_t con);
extern int setfscreatecon(*const* security_context_t context);
extern int setkeycreatecon(*const* security_context_t context);
extern int setsockcreatecon(*const* security_context_t context);
extern 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);
extern 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);
extern int security_compute_create(*const* security_context_t scon,
*const* security_context_t tcon,
security_class_t tclass,
security_context_t * newcon);
extern int security_compute_relabel(*const* security_context_t scon,
*const* security_context_t tcon,
security_class_t tclass,
security_context_t * newcon);
extern int security_compute_member(*const* security_context_t scon,
*const* security_context_t tcon,
security_class_t tclass,
security_context_t * newcon);
extern int security_compute_user(*const* security_context_t scon,
const char *username,
security_context_t ** con);
extern int security_check_context(*const* security_context_t con);
extern int security_canonicalize_context(*const* security_context_t con,
security_context_t * canoncon);
... and all the _raw version.
extern int selinux_trans_to_raw_context(*const* security_context_t trans,
security_context_t * rawp);
extern int selinux_raw_to_trans_context(*const* security_context_t raw,
security_context_t * transp);
--
KaiGai Kohei <kaigai@ak.jp.nec.com>
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: libselinux APIs should take "const" qualifier?
2010-03-19 7:52 libselinux APIs should take "const" qualifier? KaiGai Kohei
@ 2010-03-19 13:32 ` Stephen Smalley
2010-03-23 2:56 ` KaiGai Kohei
0 siblings, 1 reply; 6+ messages in thread
From: Stephen Smalley @ 2010-03-19 13:32 UTC (permalink / raw)
To: KaiGai Kohei; +Cc: SELinux
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.
> extern int setcon(*const* security_context_t con);
> extern int setexeccon(*const* security_context_t con);
> extern int setfscreatecon(*const* security_context_t context);
> extern int setkeycreatecon(*const* security_context_t context);
> extern int setsockcreatecon(*const* security_context_t context);
>
> extern 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);
> extern 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);
> extern int security_compute_create(*const* security_context_t scon,
> *const* security_context_t tcon,
> security_class_t tclass,
> security_context_t * newcon);
> extern int security_compute_relabel(*const* security_context_t scon,
> *const* security_context_t tcon,
> security_class_t tclass,
> security_context_t * newcon);
> extern int security_compute_member(*const* security_context_t scon,
> *const* security_context_t tcon,
> security_class_t tclass,
> security_context_t * newcon);
> extern int security_compute_user(*const* security_context_t scon,
> const char *username,
> security_context_t ** con);
> extern int security_check_context(*const* security_context_t con);
> extern int security_canonicalize_context(*const* security_context_t con,
> security_context_t * canoncon);
> ... and all the _raw version.
>
> extern int selinux_trans_to_raw_context(*const* security_context_t trans,
> security_context_t * rawp);
> extern int selinux_raw_to_trans_context(*const* security_context_t raw,
> security_context_t * transp);
>
--
Stephen Smalley
National Security Agency
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: libselinux APIs should take "const" qualifier?
2010-03-19 13:32 ` Stephen Smalley
@ 2010-03-23 2:56 ` KaiGai Kohei
2010-06-11 6:35 ` KaiGai Kohei
2010-06-14 20:37 ` Chad Sellers
0 siblings, 2 replies; 6+ messages in thread
From: KaiGai Kohei @ 2010-03-23 2:56 UTC (permalink / raw)
To: Stephen Smalley; +Cc: SELinux
[-- Attachment #1: Type: text/plain, Size: 3067 bytes --]
(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>
[-- Attachment #2: libselinux-const-qualify.1.patch --]
[-- Type: application/octect-stream, Size: 28020 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: libselinux APIs should take "const" qualifier?
2010-03-23 2:56 ` KaiGai Kohei
@ 2010-06-11 6:35 ` KaiGai Kohei
2010-06-14 13:50 ` Steve Lawrence
2010-06-14 20:37 ` Chad Sellers
1 sibling, 1 reply; 6+ messages in thread
From: KaiGai Kohei @ 2010-06-11 6:35 UTC (permalink / raw)
To: SELinux; +Cc: Stephen Smalley
BTW, is nobody interested in this patch?
(2010/03/23 11:56), KaiGai Kohei 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.
--
KaiGai Kohei <kaigai@ak.jp.nec.com>
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: libselinux APIs should take "const" qualifier?
2010-06-11 6:35 ` KaiGai Kohei
@ 2010-06-14 13:50 ` Steve Lawrence
0 siblings, 0 replies; 6+ messages in thread
From: Steve Lawrence @ 2010-06-14 13:50 UTC (permalink / raw)
To: KaiGai Kohei; +Cc: SELinux
On Tue, 2010-03-23 at 11:56:36 +0900, KaiGai Kohei 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.
Reviewed-by: Steve Lawrence <slawrence@tresys.com>
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: libselinux APIs should take "const" qualifier?
2010-03-23 2:56 ` KaiGai Kohei
2010-06-11 6:35 ` KaiGai Kohei
@ 2010-06-14 20:37 ` Chad Sellers
1 sibling, 0 replies; 6+ messages in thread
From: Chad Sellers @ 2010-06-14 20:37 UTC (permalink / raw)
To: KaiGai Kohei, Stephen Smalley; +Cc: SELinux
On 3/22/10 10:56 PM, "KaiGai Kohei" <kaigai@ak.jp.nec.com> 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@tresys.com>
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@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-06-14 20:37 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-03-19 7:52 libselinux APIs should take "const" qualifier? KaiGai Kohei
2010-03-19 13:32 ` Stephen Smalley
2010-03-23 2:56 ` KaiGai Kohei
2010-06-11 6:35 ` KaiGai Kohei
2010-06-14 13:50 ` Steve Lawrence
2010-06-14 20:37 ` Chad Sellers
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.