* [PATCH] libsepol users in non-mls policy bugfix
@ 2006-10-30 13:59 Joshua Brindle
2006-10-30 18:26 ` Karl MacMillan
0 siblings, 1 reply; 8+ messages in thread
From: Joshua Brindle @ 2006-10-30 13:59 UTC (permalink / raw)
To: selinux@tycho.nsa.gov; +Cc: Stephen Smalley, Karl MacMillan, Darrel Goeddel
This patch fixes a bug where adding a user in a non-mls module would
cause a segfault at link time.
diff -pruN trunk/libsepol/src/link.c user_mls_fix/libsepol/src/link.c
--- trunk/libsepol/src/link.c 2006-10-28 15:35:26.000000000 -0400
+++ user_mls_fix/libsepol/src/link.c 2006-10-28 15:26:44.000000000 -0400
@@ -994,11 +994,15 @@ static int user_fix_callback(hashtab_key
goto cleanup;
}
- if (mls_range_convert(&user->range, &new_user->range, mod))
- goto cleanup;
+ if (mod->policy->mls) {
- if (mls_level_convert(&user->dfltlevel, &new_user->dfltlevel, mod))
- goto cleanup;
+ if (mls_range_convert(&user->range, &new_user->range, mod))
+ goto cleanup;
+
+ if (mls_level_convert(&user->dfltlevel, &new_user->dfltlevel, mod))
+ goto cleanup;
+
+ }
return 0;
--
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] 8+ messages in thread
* Re: [PATCH] libsepol users in non-mls policy bugfix
2006-10-30 13:59 [PATCH] libsepol users in non-mls policy bugfix Joshua Brindle
@ 2006-10-30 18:26 ` Karl MacMillan
2006-10-30 18:31 ` Joshua Brindle
0 siblings, 1 reply; 8+ messages in thread
From: Karl MacMillan @ 2006-10-30 18:26 UTC (permalink / raw)
To: Joshua Brindle; +Cc: selinux@tycho.nsa.gov, Stephen Smalley, Darrel Goeddel
On Mon, 2006-10-30 at 08:59 -0500, Joshua Brindle wrote:
> This patch fixes a bug where adding a user in a non-mls module would
> cause a segfault at link time.
>
>
> diff -pruN trunk/libsepol/src/link.c user_mls_fix/libsepol/src/link.c
> --- trunk/libsepol/src/link.c 2006-10-28 15:35:26.000000000 -0400
> +++ user_mls_fix/libsepol/src/link.c 2006-10-28 15:26:44.000000000 -0400
> @@ -994,11 +994,15 @@ static int user_fix_callback(hashtab_key
> goto cleanup;
> }
>
> - if (mls_range_convert(&user->range, &new_user->range, mod))
> - goto cleanup;
> + if (mod->policy->mls) {
>
> - if (mls_level_convert(&user->dfltlevel, &new_user->dfltlevel, mod))
> - goto cleanup;
> + if (mls_range_convert(&user->range, &new_user->range, mod))
> + goto cleanup;
> +
> + if (mls_level_convert(&user->dfltlevel, &new_user->dfltlevel, mod))
> + goto cleanup;
> +
> + }
>
> return 0;
>
>
This patch looks ok, but shouldn't we also stop the segfault in
mls_level_convert? The use of assert there looks like it is catching
what should be a runtime error that should cause an error message rather
than bailing.
Otherwise:
Acked by: Karl MacMillan <kmacmillan@mentalrootkit.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.
--
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] 8+ messages in thread
* RE: [PATCH] libsepol users in non-mls policy bugfix
2006-10-30 18:26 ` Karl MacMillan
@ 2006-10-30 18:31 ` Joshua Brindle
2006-10-30 18:52 ` Darrel Goeddel
2006-10-30 18:56 ` Karl MacMillan
0 siblings, 2 replies; 8+ messages in thread
From: Joshua Brindle @ 2006-10-30 18:31 UTC (permalink / raw)
To: Karl MacMillan; +Cc: selinux, Stephen Smalley, Darrel Goeddel
> From: Karl MacMillan [mailto:kmacmillan@mentalrootkit.com]
>
> On Mon, 2006-10-30 at 08:59 -0500, Joshua Brindle wrote:
> > This patch fixes a bug where adding a user in a non-mls
> module would
> > cause a segfault at link time.
> >
> >
> > diff -pruN trunk/libsepol/src/link.c
> user_mls_fix/libsepol/src/link.c
> > --- trunk/libsepol/src/link.c 2006-10-28
> 15:35:26.000000000 -0400
> > +++ user_mls_fix/libsepol/src/link.c 2006-10-28
> 15:26:44.000000000 -0400
> > @@ -994,11 +994,15 @@ static int user_fix_callback(hashtab_key
> > goto cleanup;
> > }
> >
> > - if (mls_range_convert(&user->range, &new_user->range, mod))
> > - goto cleanup;
> > + if (mod->policy->mls) {
> >
> > - if (mls_level_convert(&user->dfltlevel,
> &new_user->dfltlevel, mod))
> > - goto cleanup;
> > + if (mls_range_convert(&user->range,
> &new_user->range, mod))
> > + goto cleanup;
> > +
> > + if (mls_level_convert(&user->dfltlevel,
> &new_user->dfltlevel, mod))
> > + goto cleanup;
> > +
> > + }
> >
> > return 0;
> >
> >
>
> This patch looks ok, but shouldn't we also stop the segfault
> in mls_level_convert? The use of assert there looks like it
> is catching what should be a runtime error that should cause
> an error message rather than bailing.
>
> Otherwise:
Yea, I tried thinking of a more elegant way to do it and couldn't come
up with anything, any suggestions?
Clearly those assertions only apply to the MLS case, and we caught a
plain bug because of them, I think they are not user errors.
>
> Acked by: Karl MacMillan <kmacmillan@mentalrootkit.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.
>
>
--
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] 8+ messages in thread
* Re: [PATCH] libsepol users in non-mls policy bugfix
2006-10-30 18:31 ` Joshua Brindle
@ 2006-10-30 18:52 ` Darrel Goeddel
2006-10-30 18:56 ` Karl MacMillan
1 sibling, 0 replies; 8+ messages in thread
From: Darrel Goeddel @ 2006-10-30 18:52 UTC (permalink / raw)
To: Joshua Brindle; +Cc: Karl MacMillan, selinux, Stephen Smalley
Joshua Brindle wrote:
>>From: Karl MacMillan [mailto:kmacmillan@mentalrootkit.com]
>>
>>On Mon, 2006-10-30 at 08:59 -0500, Joshua Brindle wrote:
>>
>>>This patch fixes a bug where adding a user in a non-mls
>>
>>module would
>>
>>>cause a segfault at link time.
>>>
>>>
>>>diff -pruN trunk/libsepol/src/link.c
>>
>>user_mls_fix/libsepol/src/link.c
>>
>>>--- trunk/libsepol/src/link.c 2006-10-28
>>
>>15:35:26.000000000 -0400
>>
>>>+++ user_mls_fix/libsepol/src/link.c 2006-10-28
>>
>>15:26:44.000000000 -0400
>>
>>>@@ -994,11 +994,15 @@ static int user_fix_callback(hashtab_key
>>> goto cleanup;
>>> }
>>>
>>>- if (mls_range_convert(&user->range, &new_user->range, mod))
>>>- goto cleanup;
>>>+ if (mod->policy->mls) {
>>>
>>>- if (mls_level_convert(&user->dfltlevel,
>>
>>&new_user->dfltlevel, mod))
>>
>>>- goto cleanup;
>>>+ if (mls_range_convert(&user->range,
>>
>>&new_user->range, mod))
>>
>>>+ goto cleanup;
>>>+
>>>+ if (mls_level_convert(&user->dfltlevel,
>>
>>&new_user->dfltlevel, mod))
>>
>>>+ goto cleanup;
>>>+
>>>+ }
>>>
>>> return 0;
>>>
>>>
>>
>>This patch looks ok, but shouldn't we also stop the segfault
>>in mls_level_convert? The use of assert there looks like it
>>is catching what should be a runtime error that should cause
>>an error message rather than bailing.
>>
>>Otherwise:
>
>
> Yea, I tried thinking of a more elegant way to do it and couldn't come
> up with anything, any suggestions?
>
> Clearly those assertions only apply to the MLS case, and we caught a
> plain bug because of them, I think they are not user errors.
How about just putting the mls check in mls_level_convert (as it is in
mls_semantic_level_expand())?
diff -ruNp --exclude=.svn selinux-base/libsepol/src/link.c selinux/libsepol/src/link.c
--- selinux-base/libsepol/src/link.c 2006-10-30 12:48:21.000000000 -0600
+++ selinux/libsepol/src/link.c 2006-10-30 12:45:08.000000000 -0600
@@ -832,6 +832,9 @@ static int mls_level_convert(mls_semanti
{
mls_semantic_cat_t *src_cat, *new_cat;
+ if (!mod->policy->mls)
+ return 0;
+
assert(mod->map[SYM_LEVELS][src->sens - 1]);
dst->sens = mod->map[SYM_LEVELS][src->sens - 1];
That will catch all callers of the mls conversion functions in the linker. The
asserts will then only apply in the MLS case as they should be.
--
Darrel
--
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] 8+ messages in thread
* RE: [PATCH] libsepol users in non-mls policy bugfix
2006-10-30 18:31 ` Joshua Brindle
2006-10-30 18:52 ` Darrel Goeddel
@ 2006-10-30 18:56 ` Karl MacMillan
2006-10-30 21:41 ` Joshua Brindle
1 sibling, 1 reply; 8+ messages in thread
From: Karl MacMillan @ 2006-10-30 18:56 UTC (permalink / raw)
To: Joshua Brindle; +Cc: selinux, Stephen Smalley, Darrel Goeddel
On Mon, 2006-10-30 at 13:31 -0500, Joshua Brindle wrote:
> > From: Karl MacMillan [mailto:kmacmillan@mentalrootkit.com]
> >
> > This patch looks ok, but shouldn't we also stop the segfault
> > in mls_level_convert? The use of assert there looks like it
> > is catching what should be a runtime error that should cause
> > an error message rather than bailing.
> >
> > Otherwise:
>
> Yea, I tried thinking of a more elegant way to do it and couldn't come
> up with anything, any suggestions?
>
You can put the check in mls_level_convert.
> Clearly those assertions only apply to the MLS case, and we caught a
> plain bug because of them, I think they are not user errors.
>
Yea - I forgot how all of those assertions are used in the link code.
The alternative is below (with some cleanup up error handling that makes
it look larger). Honestly, I don't care which patch.
Karl
Index: libsepol/src/link.c
===================================================================
--- libsepol/src/link.c (revision 2074)
+++ libsepol/src/link.c (working copy)
@@ -827,19 +827,24 @@
return -1;
}
-static int mls_level_convert(mls_semantic_level_t * src,
- mls_semantic_level_t * dst, policy_module_t * mod)
+static int mls_level_convert(mls_semantic_level_t * src, mls_semantic_level_t * dst,
+ policy_module_t * mod, link_state_t * state)
{
mls_semantic_cat_t *src_cat, *new_cat;
+ if (!mod->policy->mls)
+ return 0;
+
assert(mod->map[SYM_LEVELS][src->sens - 1]);
dst->sens = mod->map[SYM_LEVELS][src->sens - 1];
for (src_cat = src->cat; src_cat; src_cat = src_cat->next) {
new_cat =
(mls_semantic_cat_t *) malloc(sizeof(mls_semantic_cat_t));
- if (!new_cat)
+ if (!new_cat) {
+ ERR(state->handle, "Out of memory");
return -1;
+ }
mls_semantic_cat_init(new_cat);
new_cat->next = dst->cat;
@@ -854,13 +859,16 @@
return 0;
}
-static int mls_range_convert(mls_semantic_range_t * src,
- mls_semantic_range_t * dst, policy_module_t * mod)
+static int mls_range_convert(mls_semantic_range_t * src, mls_semantic_range_t * dst,
+ policy_module_t * mod, link_state_t * state)
{
- if (mls_level_convert(&src->level[0], &dst->level[0], mod))
- return -1;
- if (mls_level_convert(&src->level[1], &dst->level[1], mod))
- return -1;
+ int ret;
+ ret = mls_level_convert(&src->level[0], &dst->level[0], mod, state);
+ if (ret)
+ return ret;
+ ret = mls_level_convert(&src->level[1], &dst->level[1], mod, state);
+ if (ret)
+ return ret;
return 0;
}
@@ -994,10 +1002,10 @@
goto cleanup;
}
- if (mls_range_convert(&user->range, &new_user->range, mod))
+ if (mls_range_convert(&user->range, &new_user->range, mod, state))
goto cleanup;
- if (mls_level_convert(&user->dfltlevel, &new_user->dfltlevel, mod))
+ if (mls_level_convert(&user->dfltlevel, &new_user->dfltlevel, mod, state))
goto cleanup;
return 0;
@@ -1224,7 +1232,7 @@
}
}
- if (mls_range_convert(&rule->trange, &new_rule->trange, mod))
+ if (mls_range_convert(&rule->trange, &new_rule->trange, mod, state))
goto cleanup;
}
return 0;
--
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] 8+ messages in thread
* RE: [PATCH] libsepol users in non-mls policy bugfix
2006-10-30 18:56 ` Karl MacMillan
@ 2006-10-30 21:41 ` Joshua Brindle
2006-10-31 18:03 ` Stephen Smalley
0 siblings, 1 reply; 8+ messages in thread
From: Joshua Brindle @ 2006-10-30 21:41 UTC (permalink / raw)
To: Karl MacMillan; +Cc: selinux, Stephen Smalley, Darrel Goeddel
> From: Karl MacMillan [mailto:kmacmillan@mentalrootkit.com]
>
> On Mon, 2006-10-30 at 13:31 -0500, Joshua Brindle wrote:
> > > From: Karl MacMillan [mailto:kmacmillan@mentalrootkit.com]
> > >
> > > This patch looks ok, but shouldn't we also stop the segfault in
> > > mls_level_convert? The use of assert there looks like it
> is catching
> > > what should be a runtime error that should cause an error message
> > > rather than bailing.
> > >
> > > Otherwise:
> >
> > Yea, I tried thinking of a more elegant way to do it and
> couldn't come
> > up with anything, any suggestions?
> >
>
> You can put the check in mls_level_convert.
>
> > Clearly those assertions only apply to the MLS case, and we
> caught a
> > plain bug because of them, I think they are not user errors.
> >
>
> Yea - I forgot how all of those assertions are used in the link code.
> The alternative is below (with some cleanup up error handling
> that makes it look larger). Honestly, I don't care which patch.
>
Looks fine aside from the unrelated parts which should have been split
out :)
> Karl
>
>
> Index: libsepol/src/link.c
> ===================================================================
> --- libsepol/src/link.c (revision 2074)
> +++ libsepol/src/link.c (working copy)
> @@ -827,19 +827,24 @@
> return -1;
> }
>
> -static int mls_level_convert(mls_semantic_level_t * src,
> - mls_semantic_level_t * dst,
> policy_module_t * mod)
> +static int mls_level_convert(mls_semantic_level_t * src,
> mls_semantic_level_t * dst,
> + policy_module_t * mod,
> link_state_t * state)
> {
> mls_semantic_cat_t *src_cat, *new_cat;
>
> + if (!mod->policy->mls)
> + return 0;
> +
> assert(mod->map[SYM_LEVELS][src->sens - 1]);
> dst->sens = mod->map[SYM_LEVELS][src->sens - 1];
>
> for (src_cat = src->cat; src_cat; src_cat = src_cat->next) {
> new_cat =
> (mls_semantic_cat_t *)
> malloc(sizeof(mls_semantic_cat_t));
> - if (!new_cat)
> + if (!new_cat) {
> + ERR(state->handle, "Out of memory");
> return -1;
> + }
> mls_semantic_cat_init(new_cat);
>
> new_cat->next = dst->cat;
> @@ -854,13 +859,16 @@
> return 0;
> }
>
> -static int mls_range_convert(mls_semantic_range_t * src,
> - mls_semantic_range_t * dst,
> policy_module_t * mod)
> +static int mls_range_convert(mls_semantic_range_t * src,
> mls_semantic_range_t * dst,
> + policy_module_t * mod,
> link_state_t * state)
> {
> - if (mls_level_convert(&src->level[0], &dst->level[0], mod))
> - return -1;
> - if (mls_level_convert(&src->level[1], &dst->level[1], mod))
> - return -1;
> + int ret;
> + ret = mls_level_convert(&src->level[0], &dst->level[0],
> mod, state);
> + if (ret)
> + return ret;
> + ret = mls_level_convert(&src->level[1], &dst->level[1],
> mod, state);
> + if (ret)
> + return ret;
> return 0;
> }
>
> @@ -994,10 +1002,10 @@
> goto cleanup;
> }
>
> - if (mls_range_convert(&user->range, &new_user->range, mod))
> + if (mls_range_convert(&user->range, &new_user->range,
> mod, state))
> goto cleanup;
>
> - if (mls_level_convert(&user->dfltlevel,
> &new_user->dfltlevel, mod))
> + if (mls_level_convert(&user->dfltlevel,
> &new_user->dfltlevel, mod,
> +state))
> goto cleanup;
>
> return 0;
> @@ -1224,7 +1232,7 @@
> }
> }
>
> - if (mls_range_convert(&rule->trange,
> &new_rule->trange, mod))
> + if (mls_range_convert(&rule->trange,
> &new_rule->trange, mod, state))
> goto cleanup;
> }
> return 0;
>
>
>
Acked-By: Joshua Brindle <jbrindle@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] 8+ messages in thread
* RE: [PATCH] libsepol users in non-mls policy bugfix
2006-10-30 21:41 ` Joshua Brindle
@ 2006-10-31 18:03 ` Stephen Smalley
2006-10-31 18:40 ` Joshua Brindle
0 siblings, 1 reply; 8+ messages in thread
From: Stephen Smalley @ 2006-10-31 18:03 UTC (permalink / raw)
To: Joshua Brindle; +Cc: Karl MacMillan, selinux, Darrel Goeddel
On Mon, 2006-10-30 at 16:41 -0500, Joshua Brindle wrote:
> > From: Karl MacMillan [mailto:kmacmillan@mentalrootkit.com]
> >
> > On Mon, 2006-10-30 at 13:31 -0500, Joshua Brindle wrote:
> > > > From: Karl MacMillan [mailto:kmacmillan@mentalrootkit.com]
> > > >
> > > > This patch looks ok, but shouldn't we also stop the segfault in
> > > > mls_level_convert? The use of assert there looks like it
> > is catching
> > > > what should be a runtime error that should cause an error message
> > > > rather than bailing.
> > > >
> > > > Otherwise:
> > >
> > > Yea, I tried thinking of a more elegant way to do it and
> > couldn't come
> > > up with anything, any suggestions?
> > >
> >
> > You can put the check in mls_level_convert.
> >
> > > Clearly those assertions only apply to the MLS case, and we
> > caught a
> > > plain bug because of them, I think they are not user errors.
> > >
> >
> > Yea - I forgot how all of those assertions are used in the link code.
> > The alternative is below (with some cleanup up error handling
> > that makes it look larger). Honestly, I don't care which patch.
> >
>
> Looks fine aside from the unrelated parts which should have been split
> out :)
>
> > Karl
> >
> >
> > Index: libsepol/src/link.c
> > ===================================================================
> > --- libsepol/src/link.c (revision 2074)
> > +++ libsepol/src/link.c (working copy)
> > @@ -827,19 +827,24 @@
> > return -1;
> > }
> >
> > -static int mls_level_convert(mls_semantic_level_t * src,
> > - mls_semantic_level_t * dst,
> > policy_module_t * mod)
> > +static int mls_level_convert(mls_semantic_level_t * src,
> > mls_semantic_level_t * dst,
> > + policy_module_t * mod,
> > link_state_t * state)
> > {
> > mls_semantic_cat_t *src_cat, *new_cat;
> >
> > + if (!mod->policy->mls)
> > + return 0;
> > +
> > assert(mod->map[SYM_LEVELS][src->sens - 1]);
> > dst->sens = mod->map[SYM_LEVELS][src->sens - 1];
> >
> > for (src_cat = src->cat; src_cat; src_cat = src_cat->next) {
> > new_cat =
> > (mls_semantic_cat_t *)
> > malloc(sizeof(mls_semantic_cat_t));
> > - if (!new_cat)
> > + if (!new_cat) {
> > + ERR(state->handle, "Out of memory");
> > return -1;
> > + }
> > mls_semantic_cat_init(new_cat);
> >
> > new_cat->next = dst->cat;
> > @@ -854,13 +859,16 @@
> > return 0;
> > }
> >
> > -static int mls_range_convert(mls_semantic_range_t * src,
> > - mls_semantic_range_t * dst,
> > policy_module_t * mod)
> > +static int mls_range_convert(mls_semantic_range_t * src,
> > mls_semantic_range_t * dst,
> > + policy_module_t * mod,
> > link_state_t * state)
> > {
> > - if (mls_level_convert(&src->level[0], &dst->level[0], mod))
> > - return -1;
> > - if (mls_level_convert(&src->level[1], &dst->level[1], mod))
> > - return -1;
> > + int ret;
> > + ret = mls_level_convert(&src->level[0], &dst->level[0],
> > mod, state);
> > + if (ret)
> > + return ret;
> > + ret = mls_level_convert(&src->level[1], &dst->level[1],
> > mod, state);
> > + if (ret)
> > + return ret;
> > return 0;
> > }
> >
> > @@ -994,10 +1002,10 @@
> > goto cleanup;
> > }
> >
> > - if (mls_range_convert(&user->range, &new_user->range, mod))
> > + if (mls_range_convert(&user->range, &new_user->range,
> > mod, state))
> > goto cleanup;
> >
> > - if (mls_level_convert(&user->dfltlevel,
> > &new_user->dfltlevel, mod))
> > + if (mls_level_convert(&user->dfltlevel,
> > &new_user->dfltlevel, mod,
> > +state))
> > goto cleanup;
> >
> > return 0;
> > @@ -1224,7 +1232,7 @@
> > }
> > }
> >
> > - if (mls_range_convert(&rule->trange,
> > &new_rule->trange, mod))
> > + if (mls_range_convert(&rule->trange,
> > &new_rule->trange, mod, state))
> > goto cleanup;
> > }
> > return 0;
> >
> >
> >
>
> Acked-By: Joshua Brindle <jbrindle@tresys.com>
Acked-by: Stephen Smalley <sds@tycho.nsa.gov>
--
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] 8+ messages in thread
* RE: [PATCH] libsepol users in non-mls policy bugfix
2006-10-31 18:03 ` Stephen Smalley
@ 2006-10-31 18:40 ` Joshua Brindle
0 siblings, 0 replies; 8+ messages in thread
From: Joshua Brindle @ 2006-10-31 18:40 UTC (permalink / raw)
To: Stephen Smalley; +Cc: Karl MacMillan, selinux, Darrel Goeddel
> From: Stephen Smalley [mailto:sds@tycho.nsa.gov]
>
> On Mon, 2006-10-30 at 16:41 -0500, Joshua Brindle wrote:
> > > From: Karl MacMillan [mailto:kmacmillan@mentalrootkit.com]
> > >
> > > On Mon, 2006-10-30 at 13:31 -0500, Joshua Brindle wrote:
> > > > > From: Karl MacMillan [mailto:kmacmillan@mentalrootkit.com]
> > > > >
> > > > > This patch looks ok, but shouldn't we also stop the
> segfault in
> > > > > mls_level_convert? The use of assert there looks like it
> > > is catching
> > > > > what should be a runtime error that should cause an error
> > > > > message rather than bailing.
> > > > >
> > > > > Otherwise:
> > > >
> > > > Yea, I tried thinking of a more elegant way to do it and
> > > couldn't come
> > > > up with anything, any suggestions?
> > > >
> > >
> > > You can put the check in mls_level_convert.
> > >
> > > > Clearly those assertions only apply to the MLS case, and we
> > > caught a
> > > > plain bug because of them, I think they are not user errors.
> > > >
> > >
> > > Yea - I forgot how all of those assertions are used in
> the link code.
> > > The alternative is below (with some cleanup up error
> handling that
> > > makes it look larger). Honestly, I don't care which patch.
> > >
> >
> > Looks fine aside from the unrelated parts which should have
> been split
> > out :)
> >
> > > Karl
> > >
> > >
> > > Index: libsepol/src/link.c
> > >
> ===================================================================
> > > --- libsepol/src/link.c (revision 2074)
> > > +++ libsepol/src/link.c (working copy)
> > > @@ -827,19 +827,24 @@
> > > return -1;
> > > }
> > >
> > > -static int mls_level_convert(mls_semantic_level_t * src,
> > > - mls_semantic_level_t * dst,
> > > policy_module_t * mod)
> > > +static int mls_level_convert(mls_semantic_level_t * src,
> > > mls_semantic_level_t * dst,
> > > + policy_module_t * mod,
> > > link_state_t * state)
> > > {
> > > mls_semantic_cat_t *src_cat, *new_cat;
> > >
> > > + if (!mod->policy->mls)
> > > + return 0;
> > > +
> > > assert(mod->map[SYM_LEVELS][src->sens - 1]);
> > > dst->sens = mod->map[SYM_LEVELS][src->sens - 1];
> > >
> > > for (src_cat = src->cat; src_cat; src_cat = src_cat->next) {
> > > new_cat =
> > > (mls_semantic_cat_t *)
> > > malloc(sizeof(mls_semantic_cat_t));
> > > - if (!new_cat)
> > > + if (!new_cat) {
> > > + ERR(state->handle, "Out of memory");
> > > return -1;
> > > + }
> > > mls_semantic_cat_init(new_cat);
> > >
> > > new_cat->next = dst->cat;
> > > @@ -854,13 +859,16 @@
> > > return 0;
> > > }
> > >
> > > -static int mls_range_convert(mls_semantic_range_t * src,
> > > - mls_semantic_range_t * dst,
> > > policy_module_t * mod)
> > > +static int mls_range_convert(mls_semantic_range_t * src,
> > > mls_semantic_range_t * dst,
> > > + policy_module_t * mod,
> > > link_state_t * state)
> > > {
> > > - if (mls_level_convert(&src->level[0], &dst->level[0], mod))
> > > - return -1;
> > > - if (mls_level_convert(&src->level[1], &dst->level[1], mod))
> > > - return -1;
> > > + int ret;
> > > + ret = mls_level_convert(&src->level[0], &dst->level[0],
> > > mod, state);
> > > + if (ret)
> > > + return ret;
> > > + ret = mls_level_convert(&src->level[1], &dst->level[1],
> > > mod, state);
> > > + if (ret)
> > > + return ret;
> > > return 0;
> > > }
> > >
> > > @@ -994,10 +1002,10 @@
> > > goto cleanup;
> > > }
> > >
> > > - if (mls_range_convert(&user->range, &new_user->range, mod))
> > > + if (mls_range_convert(&user->range, &new_user->range,
> > > mod, state))
> > > goto cleanup;
> > >
> > > - if (mls_level_convert(&user->dfltlevel,
> > > &new_user->dfltlevel, mod))
> > > + if (mls_level_convert(&user->dfltlevel,
> > > &new_user->dfltlevel, mod,
> > > +state))
> > > goto cleanup;
> > >
> > > return 0;
> > > @@ -1224,7 +1232,7 @@
> > > }
> > > }
> > >
> > > - if (mls_range_convert(&rule->trange,
> > > &new_rule->trange, mod))
> > > + if (mls_range_convert(&rule->trange,
> > > &new_rule->trange, mod, state))
> > > goto cleanup;
> > > }
> > > return 0;
> > >
> > >
> > >
> >
> > Acked-By: Joshua Brindle <jbrindle@tresys.com>
>
> Acked-by: Stephen Smalley <sds@tycho.nsa.gov>
>
Merged as of libsepol 1.15.2
--
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] 8+ messages in thread
end of thread, other threads:[~2006-10-31 18:40 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-10-30 13:59 [PATCH] libsepol users in non-mls policy bugfix Joshua Brindle
2006-10-30 18:26 ` Karl MacMillan
2006-10-30 18:31 ` Joshua Brindle
2006-10-30 18:52 ` Darrel Goeddel
2006-10-30 18:56 ` Karl MacMillan
2006-10-30 21:41 ` Joshua Brindle
2006-10-31 18:03 ` Stephen Smalley
2006-10-31 18:40 ` Joshua Brindle
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.