netdev - Re: speaking of stacks

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 04 Apr 2008 08:24:12 -0400
From:	jamal <hadi@...erus.ca>
To:	David Miller <davem@...emloft.net>
Cc:	netdev@...r.kernel.org, joern@...ybastard.org,
	herbert@...dor.apana.org.au, Paul Moore <paul.moore@...com>,
	James Morris <jmorris@...ei.org>
Subject: Re: speaking of stacks

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

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html