Re: speaking of stacks

[jamal <hadi@cyberus.ca>]

On Thu, 2008-03-04 at 14:18 -0700, David Miller wrote:

> You probably have most of the security infrastructure turned
> off, therefore GCC can see that 'tmp' is basically unused
> and can therefore be totally eliminated.
> 
> The memset() call makes 'tmp' get passed by reference to
> another function, and thus become used.

Indeed, thanks - that resolves the mystery;->

Testing by moving the tmp memseting inside security_xfrm_policy_alloc()
so memset is only invoked when CONFIG_SECURITY_NETWORK_XFRM resolves the
stack abuse.
BTW, of the top 10 stack abusers _in the kernel_ (should say based on my
config) constitute 3-4 spots which are caused by this exact thing.
I could send a patch that resolves the issue by moving memset but that
would only fix it for people like myself who turn off SELinux.

> This whole song and dance here is for SELINUX to set only
> the policy->security, so that we can pass that back down
> into the subsequent xfrm_policy_bysel_ctx().
> 
> The thing to do is to rearrange these security layer hooks
> so that they take a "struct xfrm_sec_ctx **" instead of
> a full policy pointer.
> 
> Then the code would look like:
> 
> 		struct nlattr *rt = attrs[XFRMA_SEC_CTX];
> 		struct xfrm_sec_ctx *ctx;
> 
> 		err = verify_sec_ctx_len(attrs);
> 		if (err)
> 			return err;
> 
> 		if (rt) {
> 			struct xfrm_user_sec_ctx *uctx = nla_data(rt);
> 
> 			if ((err = security_xfrm_policy_alloc(&ctx, uctx)))
> 				return err;
> 		}
> 		xp = xfrm_policy_bysel_ctx(type, p->dir, &p->sel, ctx,
> 					   delete, &err);
> 		security_xfrm_policy_free(ctx);
> 
> And thus the xfrm_policy wouldn't need to be on the stack
> any longer.

Yes, that would be cleaner than what i did; i will give the opportunity
to the SELinux folks to take a first crack at it with the above
approach. 

CCing some of the SElinux folks. 
Thanks Dave.

cheers,
jamal