Stephen Smalley wrote:
> On Mon, 2006-10-02 at 12:12 -0400, Paul Moore wrote:
> 
>>Venkat Yekkirala wrote:
>>
>>>This defines SELinux enforcement of the 2 new LSM hooks as well
>>>as related changes elsewhere in the SELinux code.
>>>
>>>This also now keeps track of the peersid thru the establishment
>>>of a connection on the server (tracking peersid on the client
>>>is covered later in this patch set).
>>>
>>>Signed-off-by: Venkat Yekkirala <[EMAIL PROTECTED]>
>>>
>>>{snip}
>>>
>>>+static int selinux_skb_flow_in(struct sk_buff *skb, unsigned short family)
>>>+{
>>>+    u32 xfrm_sid;
>>>+    int err;
>>>+
>>>+    if (selinux_compat_net)
>>>+            return 1;
>>>+
>>>+    /*
>>>+     * loopback traffic already labeled and
>>>+     * flow-controlled on outbound. We may
>>>+     * need to flow-control on the inbound
>>>+     * as well if there's ever a use-case for it.
>>>+     */
>>>+    if (skb->dev == &loopback_dev)
>>>+            return 1;
>>>+
>>>+    err = selinux_xfrm_decode_session(skb, &xfrm_sid, 0);
>>>+    BUG_ON(err);
>>
>>Just a quick question that has been nagging me for awhile - any
>>particular reason why this is a BUG_ON() and not an "if (err) goto out;"?
>  
> It appears that selinux_xfrm_decode_session() can only legitimately
> return an error if the last argument (ckall) is non-zero.
> security_skb_classify_flow() was doing the same thing prior to this
> patch series.  It would be clearer if there were two separate interfaces
> that internally use the same helper, with one of the functions returning
> void.

My immediate concern is not really what selinux_xfrm_decode_session()
returns, but how to handle it, or rather errors in general, in
selinux_skb_flow_in().  I'm in the process of creating a patch to add
the missing NetLabel support to the secid patches and I am wondering if
I should BUG_ON() for an error condition or simply jump to "out".
Jumping seems a bit cleaner to me, although perhaps harder to debug, so
I was just wondering what the reasoning was behind the use of BUG_ON().

I honestly don't care at this point, it's a rather minor detail, I'd
just like to "do the right thing" with the NetLabel patch.

-- 
paul moore
linux security @ hp
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to