On Mon, 23 Jan 2006, Catherine Zhang wrote:
> +static int selinux_socket_getpeersec_udp(struct sk_buff *skb, char
> **scontext, u32 *scontext_len)
> +{
> + int err = 0;
> + u32 peer_sid = selinux_socket_getpeer_udp(skb);
> +
> + if (!peer_sid)
> + return -EINVAL;
> +
> + err = security_sid_to_context(peer_sid, scontext, scontext_len);
> + if (err)
> + return -EINVAL;
All of these types of error returns need to be fixed to propagate the
actual error encountered rather than these catch-all values.
In the cases where you're returning a u32 sid, I'd suggest passing a
pointer to the sid to store the value of the sid, and use an int return to
propagate error values.
e.g.
> +u32 selinux_socket_getpeer(struct sock *sk);
> +u32 selinux_socket_getpeer_udp(struct sk_buff *skb);
should be
int selinux_socket_getpeer(struct sock *sk, u32 *sid);
int selinux_socket_getpeer_udp(struct sk_buff *skb, u32 *sid);
Also, don't hardcode checks for zero value SIDs, use SECSID_NULL.
> int (*socket_getpeersec) (struct socket *sock, char __user *optval, int
> __user *optlen, unsigned len);
> + int (*socket_getpeersec_udp) (struct sk_buff *skb, char **scontext, u32
> *scontext_len);
These hooks should be called:
socket_getpeersec_stream
socket_getpeersec_dgram
(and similar in the SELinux code).
The parameters should be less SELinux-specific, e.g. secdata, seclen.
Also, can this support Unix datagram sockets now?
> - if (((1<<optname) & ((1<<IP_PKTINFO) | (1<<IP_RECVTTL) |
> - (1<<IP_RECVOPTS) | (1<<IP_RECVTOS) |
> - (1<<IP_RETOPTS) | (1<<IP_TOS) |
> - (1<<IP_TTL) | (1<<IP_HDRINCL) |
> - (1<<IP_MTU_DISCOVER) | (1<<IP_RECVERR) |
> - (1<<IP_ROUTER_ALERT) | (1<<IP_FREEBIND))) ||
> - optname == IP_MULTICAST_TTL ||
> - optname == IP_MULTICAST_LOOP) {
> + if (((1<<optname) & ((1<<IP_PKTINFO) | (1<<IP_RECVTTL) |
> + (1<<IP_RECVOPTS) | (1<<IP_RECVTOS) |
> + (1<<IP_RETOPTS) | (1<<IP_PASSSEC) | (1<<IP_TOS) |
> + (1<<IP_TTL) | (1<<IP_HDRINCL) |
> + (1<<IP_MTU_DISCOVER) | (1<<IP_RECVERR) |
> + (1<<IP_ROUTER_ALERT) | (1<<IP_FREEBIND))) ||
> + optname == IP_MULTICAST_TTL ||
> + optname == IP_MULTICAST_LOOP) {
Why so many lines changed when all you did was add one new option?
Please retain two per line in keeping with the existing code and add it at
the end.
> -static int selinux_xfrm_sec_ctx_alloc(struct xfrm_sec_ctx **ctxp, struct
> xfrm_user_sec_ctx *uctx)
> +static inline int selinux_xfrm_sec_ctx_alloc(struct xfrm_sec_ctx **ctxp,
> struct xfrm_user_sec_ctx *uctx)
Why did you make this inline?
> + if (sk->sk_state != TCP_ESTABLISHED) return 0;
Kernel coding style: put returns on the next line.
> + for (i=sp->len-1; i>=0; i--) {
Kernel coding style: (i = sp->len-1; i >= 0; i--)
--
James Morris
<[EMAIL PROTECTED]>
-
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
