Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1187391 > unrolled thread

Re: [RFC PATCH 04/21] x86/hweight: Add stack frame dependency for __arch_hweight*()

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [RFC PATCH 04/21] x86/hweight: Add stack frame dependency for  __arch_hweight*() Borislav Petkov <bp@alien8.de> - 2015-07-18 07:10 +0200
    Re: [RFC PATCH 04/21] x86/hweight: Add stack frame dependency for  __arch_hweight*() Borislav Petkov <bp@alien8.de> - 2015-07-19 06:20 +0200

#1187391 — Re: [RFC PATCH 04/21] x86/hweight: Add stack frame dependency for __arch_hweight*()

On Fri, Jul 17, 2015 at 12:32:20PM -0500, Josh Poimboeuf wrote:
> Well, but this isn't some whitelist code to make stackvalidate happy.
> 
> It's actually a real runtime frame pointer bug, and the rsp dependency
> is real.  If it does the call without first creating the stack frame
> then it breaks frame pointer based stack traces.

I think we can live with the stack trace being a little wrong in those
__sw_* variants. And besides, we're talking about the very very small
percentage of machines (which keeps getting smaller) which don't
support POPCNT. And from those, only for the cases where the arg is not
__builtin_constant_p() because there we do the __const_hweight* thing.

I'd prefer to not clutter the code more in that case.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1187523

On Sat, Jul 18, 2015 at 10:57:14AM -0500, Josh Poimboeuf wrote:
> Currently, when stackvalidate sees an ALTERNATIVE, it assumes that
> either code path is possible, so it follows both paths in parallel.
> 
> If I understand right, you're proposing that stackvalidate should only
> follow the POPCNT path and never follow the !POPCNT path?

Actually, you don't even need to follow the POPCNT case either because
it is a single instruction - no stack operations there.

So yeah, either that or special-case the case where the original insn is
CALL and the replacement is a POPCNT and ignore those CALL locations.

The advantage is that the burden is put on the tool and not by adding
markers to kernel code paths.

> In general, I agree, and I like the original patch much better.  IMO, it
> achieved the goal of keeping the kernel code clean, while fixing the
> frame pointer bug.

And I think that in that case, adding that rSP dependency is too much
because even though it fixes the "bug", it is very very unlikely any
stack trace will have __sw_hweight* in it for reasons pointed out
earlier and also because those functions can't fail and they get
integral types as args which can't fail when deref-fing either. And even
if they do, they don't call any other functions so rIP pointing to them
is already enough.

So even if we're not 100% correct wrt stack traces in this case, I think
that's ok.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web