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: Windows password security audit tool. GUI, reports in PDF.

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

Date:	Thu, 03 Apr 2008 14:18:15 -0700 (PDT)
From:	David Miller <davem@...emloft.net>
To:	hadi@...erus.ca
Cc:	netdev@...r.kernel.org, joern@...ybastard.org,
	herbert@...dor.apana.org.au
Subject: Re: speaking of stacks

From: jamal <hadi@...erus.ca>
Date: Mon, 31 Mar 2008 09:15:56 -0400

> 
> All this talk about stacks got me thirsty.
> Heres something thats a mystery to me; probably very basic
> when doing static check analysis (kernel make checkstack)
> 
> I have ipsec turned on in the linus git tree kernel and
> net/xfrm/xfrm_user.c::xfrm_get_policy() shows up as one of
> the top offenders. Eyeballing reveals that
> struct xfrm_policy seems to be the offender 
> in the else {} after if (p->index) in that function.
> That seems understandable - once variable "tmp" is declared it goes
> on the stack and it is freakin huge. 
> However, checkstack only seems to catch it if i have the
> line "memset(&tmp, 0, sizeof(struct xfrm_policy))". If i remove
> that line - even though tmp is on the stack - checkstack doesnt catch
> it.
> 
> What gives?

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.

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.
--
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