<snip>
static void sk_free_security(struct sock *sk)
{
- struct sk_security_struct *ssec = sk->sk_security;
+ struct sk_security_struct *ssec;
+
+ security_del_value(sk->sk_security, SELINUX_LSM_ID, ssec);
- if (sk->sk_family != PF_UNIX || ssec->magic != SELINUX_MAGIC)
+ if (sk->sk_family != PF_UNIX)
return;
- sk->sk_security = NULL;
kfree(ssec);
}
</snip>
security_del_value() should only be called if we get past the PF_UNIX
test, as with the security_set_value_nocheck() call in
sk_alloc_security(). You cannot safely use the sk_security field with
INET sockets presently (requires one of the LSM networking changes that
didn't make it into 2.5, to correctly handle the new connection socket
creation case).
<snip>
@@ -1388,12 +1401,12 @@
static int selinux_capset_check(struct task_struct *target, kernel_cap_t *effective,
kernel_cap_t *inheritable, kernel_cap_t *permitted)
{
- int error;
-
- error = secondary_ops->capset_check(target, effective, inheritable, permitted);
- if (error)
- return error;
-
+ if (secondary_ops) {
+ int error = secondary_ops->capset_check(target, effective,
+ inheritable, permitted);
+ if (error)
+ return error;
+ }
return task_has_perm(current, target, PROCESS__SETCAP);
}
</snip>
Why did you make this conditional on secondary_ops, vs. any other hook?
SELinux should always be setting secondary_ops during initialization,
even if only to the dummy module.
<snip>
@@ -1402,6 +1415,9 @@
{
int error;
+ if (!secondary_ops)
+ return;
+
error = task_has_perm(current, target, PROCESS__SETCAP);
if (error)
return;
</snip>
As above, why add this check of secondary_ops (and further why skip the
check entirely if it is not set - it is still setting the state of
possibly another process, and that needs to be controlled by the
policy).
netif doesn't need the header, right, as it isn't an LSM field.
I'm not sure about the interface for get/set:
- I would think you would want it to be equally possible to implement
them as functions rather than macros (i.e. explicitly pass by pointer
arguments that are going to be set),
- I would think that you would want to allow existing forms like:
struct inode_security_struct *isec = inode->i_security;
to be able to be converted more easily to something like:
struct inode_security_struct *isec = security_get_value(inode, ID);
I also wonder if you should just make "security_info" into a struct
security_header that is empty (but not completely undefined) in the
#undef case and otherwise includes the necessary fields, and then insert
'struct security_header;' at the beginning of each object definition.
Seems cleaner to me.
--
Stephen Smalley <sds@private>
National Security Agency
This archive was generated by hypermail 2.1.3 : Fri Nov 12 2004 - 12:59:46 PST