Re: [PATCH v2] nfnetlink_queue: add security context information

[Florian Westphal <fw@strlen.de>]

Roman Kubiak <r.kubiak@samsung.com> wrote:
> [sidenote]
> The additional NULL at the end of the security context is there because SMACK does not add this
> to it's labels while SELinux does. So in order to avoid checking i just add it always.
> This additional byte is also represented when calculating the size.
> I did that because we are not transmitting the size of the context and there is no specified
> max length so it has to be NULL terminated (at least it seemed like a valid solution)

The netlink header contains the size of the attribute.
I'd prefer to not have the kernel deal with NULL termination.

> +static u32 nfqnl_get_sk_secctx(struct sock *sk, char **secdata)
> +{
> +	u32 secid = 0;
> +	u32 seclen = 0;
> +	int ret = -1;
> +
> +	if (!sk || !sk_fullsock(sk))
> +		return ret;

return 0/return seclen?

> +	if ((queue->flags & NFQA_CFG_F_SECCTX) && entskb->sk) {
> +		seclen = nfqnl_get_sk_secctx(entskb->sk, &secdata);
> +			if (seclen > 0)
> +				size += nla_total_size(seclen) + 1;

Wrong intent level for if (seclen > 0)

Other than this, it looks ok to me.