* Re: [PATCH 4/4] selinux: xfrm - notify users on dropped packets
[not found] ` <20110223115715.GL20852@secunet.com>
@ 2011-02-23 14:56 ` Paul Moore
2011-02-24 7:33 ` Steffen Klassert
2011-02-24 8:22 ` [PATCH 4/4 v2] " Steffen Klassert
0 siblings, 2 replies; 6+ messages in thread
From: Paul Moore @ 2011-02-23 14:56 UTC (permalink / raw)
To: Steffen Klassert; +Cc: James Morris, Eric Paris, linux-security-module, selinux
On Wednesday, February 23, 2011 6:57:15 AM Steffen Klassert wrote:
> In selinux_xfrm_state_pol_flow_match we have cases where we drop
> packets without asking the avc. No audit message is generated in
> this case. Lets at least print out a message to the logs, so the
> users don't need to dig in the code to find out why these packets
> are dropped.
>
> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
> Acked-by: Paul Moore <paul.moore@hp.com>
> ---
>
> Paul,
>
> I removed the exclamation marks and I changed this one
> to print a message too if the flow label does not match the SA label.
> I kept your ACK on this one, I hope it is ok.
> If not please complain.
I still see the exclamation marks in the patch below; perhaps you sent the
wrong version by mistake?
Also, as far as keeping ACKs on modified patches, I think it is okay as long
as you are just porting a patch, e.g. minor changes necessary so that the
patch applies to the current stable kernel. If you have to do anything
beyond that, I would appreciate it if you could drop my ACK; I don't have a
problem with re-reviewing patches and re-ACKing them if they look okay.
> security/selinux/xfrm.c | 26 +++++++++++++++++---------
> 1 files changed, 17 insertions(+), 9 deletions(-)
>
> diff --git a/security/selinux/xfrm.c b/security/selinux/xfrm.c
> index 728c57e..0062613 100644
> --- a/security/selinux/xfrm.c
> +++ b/security/selinux/xfrm.c
> @@ -118,25 +118,33 @@ int selinux_xfrm_state_pol_flow_match(struct
> xfrm_state *x, struct xfrm_policy * int rc;
>
> if (!xp->security)
> - if (x->security)
> - /* unlabeled policy and labeled SA can't match */
> + if (x->security) {
> + if (net_ratelimit())
> + printk("selinux: unlabeled policy and labeled SA can't
match!\n");
> return 0;
> - else
> + } else
> /* unlabeled policy and unlabeled SA match all flows */
> return 1;
> else
> - if (!x->security)
> - /* unlabeled SA and labeled policy can't match */
> + if (!x->security) {
> + if (net_ratelimit())
> + printk("selinux: unlabeled SA and labeled policy can't
match!\n");
> return 0;
> - else
> - if (!selinux_authorizable_xfrm(x))
> - /* Not a SELinux-labeled SA */
> + } else {
> + if (!selinux_authorizable_xfrm(x)) {
> + if (net_ratelimit())
> + printk("selinux: Not a SELinux-labeled SA!\n");
> return 0;
> + }
> + }
>
> state_sid = x->security->ctx_sid;
>
> - if (fl->secid != state_sid)
> + if (fl->secid != state_sid) {
> + if (net_ratelimit())
> + printk("selinux: Flow label does not match SA label!\n");
> return 0;
> + }
>
> rc = avc_has_perm(fl->secid, state_sid, SECCLASS_ASSOCIATION,
> ASSOCIATION__SENDTO,
--
paul moore
linux @ hp
--
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: [PATCH 4/4] selinux: xfrm - notify users on dropped packets
2011-02-23 14:56 ` [PATCH 4/4] selinux: xfrm - notify users on dropped packets Paul Moore
@ 2011-02-24 7:33 ` Steffen Klassert
2011-02-24 8:22 ` [PATCH 4/4 v2] " Steffen Klassert
1 sibling, 0 replies; 6+ messages in thread
From: Steffen Klassert @ 2011-02-24 7:33 UTC (permalink / raw)
To: Paul Moore; +Cc: James Morris, Eric Paris, linux-security-module, selinux
On Wed, Feb 23, 2011 at 09:56:52AM -0500, Paul Moore wrote:
>
> I still see the exclamation marks in the patch below; perhaps you sent the
> wrong version by mistake?
>
Argh, yes. I'll update the git branch and resend the patch.
--
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
* [PATCH 4/4 v2] selinux: xfrm - notify users on dropped packets
2011-02-23 14:56 ` [PATCH 4/4] selinux: xfrm - notify users on dropped packets Paul Moore
2011-02-24 7:33 ` Steffen Klassert
@ 2011-02-24 8:22 ` Steffen Klassert
2011-02-24 14:51 ` Eric Paris
1 sibling, 1 reply; 6+ messages in thread
From: Steffen Klassert @ 2011-02-24 8:22 UTC (permalink / raw)
To: Paul Moore; +Cc: James Morris, Eric Paris, linux-security-module, selinux
In selinux_xfrm_state_pol_flow_match we have cases where we drop
packets without asking the avc. No audit message is generated in
this case. Lets at least print out a message to the logs, so the
users don't need to dig in the code to find out why these packets
are dropped.
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
security/selinux/xfrm.c | 26 +++++++++++++++++---------
1 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/security/selinux/xfrm.c b/security/selinux/xfrm.c
index 728c57e..b0dd401 100644
--- a/security/selinux/xfrm.c
+++ b/security/selinux/xfrm.c
@@ -118,25 +118,33 @@ int selinux_xfrm_state_pol_flow_match(struct xfrm_state *x, struct xfrm_policy *
int rc;
if (!xp->security)
- if (x->security)
- /* unlabeled policy and labeled SA can't match */
+ if (x->security) {
+ if (net_ratelimit())
+ printk("selinux: unlabeled policy and labeled SA can't match\n");
return 0;
- else
+ } else
/* unlabeled policy and unlabeled SA match all flows */
return 1;
else
- if (!x->security)
- /* unlabeled SA and labeled policy can't match */
+ if (!x->security) {
+ if (net_ratelimit())
+ printk("selinux: unlabeled SA and labeled policy can't match\n");
return 0;
- else
- if (!selinux_authorizable_xfrm(x))
- /* Not a SELinux-labeled SA */
+ } else {
+ if (!selinux_authorizable_xfrm(x)) {
+ if (net_ratelimit())
+ printk("selinux: Not a SELinux-labeled SA\n");
return 0;
+ }
+ }
state_sid = x->security->ctx_sid;
- if (fl->secid != state_sid)
+ if (fl->secid != state_sid) {
+ if (net_ratelimit())
+ printk("selinux: Flow label does not match SA label\n");
return 0;
+ }
rc = avc_has_perm(fl->secid, state_sid, SECCLASS_ASSOCIATION,
ASSOCIATION__SENDTO,
--
1.7.0.4
--
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 related [flat|nested] 6+ messages in thread
* Re: [PATCH 4/4 v2] selinux: xfrm - notify users on dropped packets
2011-02-24 8:22 ` [PATCH 4/4 v2] " Steffen Klassert
@ 2011-02-24 14:51 ` Eric Paris
2011-02-25 9:56 ` Steffen Klassert
0 siblings, 1 reply; 6+ messages in thread
From: Eric Paris @ 2011-02-24 14:51 UTC (permalink / raw)
To: Steffen Klassert
Cc: Paul Moore, James Morris, Eric Paris, linux-security-module,
selinux
All printk() statement require a KERN_*. I've often heard upstream
people say that every printk should only be printed if you expect the
user to do something with it. If it doesn't give the user enough
information to fix whatever the problem is, or know how to fix
whatever the problem is, it's a bad printk. I don't know this code at
all, but I'm pretty sure if I got those printk's I'd just feel
dumb....
-Eric
On Thu, Feb 24, 2011 at 3:22 AM, Steffen Klassert
<steffen.klassert@secunet.com> wrote:
> In selinux_xfrm_state_pol_flow_match we have cases where we drop
> packets without asking the avc. No audit message is generated in
> this case. Lets at least print out a message to the logs, so the
> users don't need to dig in the code to find out why these packets
> are dropped.
>
> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
> ---
> security/selinux/xfrm.c | 26 +++++++++++++++++---------
> 1 files changed, 17 insertions(+), 9 deletions(-)
>
> diff --git a/security/selinux/xfrm.c b/security/selinux/xfrm.c
> index 728c57e..b0dd401 100644
> --- a/security/selinux/xfrm.c
> +++ b/security/selinux/xfrm.c
> @@ -118,25 +118,33 @@ int selinux_xfrm_state_pol_flow_match(struct xfrm_state *x, struct xfrm_policy *
> int rc;
>
> if (!xp->security)
> - if (x->security)
> - /* unlabeled policy and labeled SA can't match */
> + if (x->security) {
> + if (net_ratelimit())
> + printk("selinux: unlabeled policy and labeled SA can't match\n");
> return 0;
> - else
> + } else
> /* unlabeled policy and unlabeled SA match all flows */
> return 1;
> else
> - if (!x->security)
> - /* unlabeled SA and labeled policy can't match */
> + if (!x->security) {
> + if (net_ratelimit())
> + printk("selinux: unlabeled SA and labeled policy can't match\n");
> return 0;
> - else
> - if (!selinux_authorizable_xfrm(x))
> - /* Not a SELinux-labeled SA */
> + } else {
> + if (!selinux_authorizable_xfrm(x)) {
> + if (net_ratelimit())
> + printk("selinux: Not a SELinux-labeled SA\n");
> return 0;
> + }
> + }
>
> state_sid = x->security->ctx_sid;
>
> - if (fl->secid != state_sid)
> + if (fl->secid != state_sid) {
> + if (net_ratelimit())
> + printk("selinux: Flow label does not match SA label\n");
> return 0;
> + }
>
> rc = avc_has_perm(fl->secid, state_sid, SECCLASS_ASSOCIATION,
> ASSOCIATION__SENDTO,
> --
> 1.7.0.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
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: [PATCH 4/4 v2] selinux: xfrm - notify users on dropped packets
2011-02-24 14:51 ` Eric Paris
@ 2011-02-25 9:56 ` Steffen Klassert
2011-02-25 20:07 ` Eric Paris
0 siblings, 1 reply; 6+ messages in thread
From: Steffen Klassert @ 2011-02-25 9:56 UTC (permalink / raw)
To: Eric Paris
Cc: Paul Moore, James Morris, Eric Paris, linux-security-module,
selinux
On Thu, Feb 24, 2011 at 09:51:04AM -0500, Eric Paris wrote:
> All printk() statement require a KERN_*. I've often heard upstream
> people say that every printk should only be printed if you expect the
> user to do something with it. If it doesn't give the user enough
> information to fix whatever the problem is, or know how to fix
> whatever the problem is, it's a bad printk. I don't know this code at
> all, but I'm pretty sure if I got those printk's I'd just feel
> dumb....
>
The problem with this patch is, that it was one of my debugging patches
when I tried to find out why my packets are dropped. It was not
contemplated to push it upstream when I wrote it. Then I thought it
would be good to print out a message if these packets are dropped
and I picked the patch from my debugging branch, which was a bad idea.
I think we should drop this patch entirely and focus on the remaining
3 paches for now, as they are real fixes. So I could resend the remaining
patches for another round of review, or does anybody want to thake them
as they are?
Steffen
--
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: [PATCH 4/4 v2] selinux: xfrm - notify users on dropped packets
2011-02-25 9:56 ` Steffen Klassert
@ 2011-02-25 20:07 ` Eric Paris
0 siblings, 0 replies; 6+ messages in thread
From: Eric Paris @ 2011-02-25 20:07 UTC (permalink / raw)
To: Steffen Klassert
Cc: Paul Moore, James Morris, Eric Paris, linux-security-module,
selinux
Patches 1-3 have been committed to
http://git.infradead.org/users/eparis/selinux.git
thanks!
-Eric
On Fri, Feb 25, 2011 at 4:56 AM, Steffen Klassert
<steffen.klassert@secunet.com> wrote:
> On Thu, Feb 24, 2011 at 09:51:04AM -0500, Eric Paris wrote:
>> All printk() statement require a KERN_*. I've often heard upstream
>> people say that every printk should only be printed if you expect the
>> user to do something with it. If it doesn't give the user enough
>> information to fix whatever the problem is, or know how to fix
>> whatever the problem is, it's a bad printk. I don't know this code at
>> all, but I'm pretty sure if I got those printk's I'd just feel
>> dumb....
>>
>
> The problem with this patch is, that it was one of my debugging patches
> when I tried to find out why my packets are dropped. It was not
> contemplated to push it upstream when I wrote it. Then I thought it
> would be good to print out a message if these packets are dropped
> and I picked the patch from my debugging branch, which was a bad idea.
> I think we should drop this patch entirely and focus on the remaining
> 3 paches for now, as they are real fixes. So I could resend the remaining
> patches for another round of review, or does anybody want to thake them
> as they are?
>
> Steffen
>
--
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:[~2011-02-25 20:07 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20110223115343.GH20852@secunet.com>
[not found] ` <20110223115715.GL20852@secunet.com>
2011-02-23 14:56 ` [PATCH 4/4] selinux: xfrm - notify users on dropped packets Paul Moore
2011-02-24 7:33 ` Steffen Klassert
2011-02-24 8:22 ` [PATCH 4/4 v2] " Steffen Klassert
2011-02-24 14:51 ` Eric Paris
2011-02-25 9:56 ` Steffen Klassert
2011-02-25 20:07 ` Eric Paris
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.