On 2018-06-11 09:38, David Holmes wrote:
Hi Magnus,
On 11/06/2018 5:10 PM, Magnus Ihse Bursie wrote:
On 2018-06-08 23:50, Erik Joelsson wrote:
On 2018-06-07 17:30, David Holmes wrote:
On 8/06/2018 6:11 AM, Erik Joelsson wrote:
I just don't think the extra work is warranted or should be
prioritized at this point. I also cannot think of a combination of
options required for what you are suggesting that wouldn't be
confusing to the user. If someone truly feels like these flags are
forced on them and can't live with them, we or preferably that
person can fix it then. I don't think that's dictatorship. OpenJDK
is still open source and anyone can contribute.
I don't see why --enable-hardened-jdk and --enable-hardened-hotspot
to add to the right flags would be either complicated or confusing.
For me the confusion surrounds the difference between
--enable-hardened-hotspot and --with-jvm-variants=server, hardened
and making the user understand it. But sure, it is doable. Here is a
new webrev with those two options as I interpret them. Here is the
help text:
--enable-hardened-jdk enable hardenening compiler flags for all jdk
libraries (except the JVM), typically
disabling
speculative cti. [disabled]
--enable-hardened-hotspot
enable hardenening compiler flags for
hotspot (all
jvm variants), typically disabling
speculative cti.
To make hardening of hotspot a runtime
choice,
consider the "hardened" jvm variant
instead of this
option. [disabled]
Note that this changes the default for jdk libraries to not enable
hardening unless the user requests it.
Webrev: http://cr.openjdk.java.net/~erikj/8202384/webrev.04/
Hold it, hold it! I'm not sure how we ended up here, but I don't like
it at all. :-(
I think Eriks initial patch is much better than this. Some arguments
in random order to defend this position:
1) Why should we have a configure option to disable security relevant
flags for the JDK, if there has been no measured negative effect? We
don't do this for any other compiler flags, especially not security
relevant ones!
I've re-read the entire thread to see if I could understand what
could possibly motivate this, but the only thing I can find is David
Holmes vague fear that these flags would not be well-tested enough.
Let me counter with my own vague guesses: I believe the spectre
mitigation methods to have been fully and properly tested, since they
are rolled-out massively on all products. And let me complement with
my own fear: the PR catastrophe if OpenJDK were *not* built with
spectre mitigations, and someone were to exploit that!
All I'm looking for is the ability to select whether you can build
with or without this "hardening". The default OpenJDK build can of
course churn out a "hardened" implementation. Anyone who opts out of
that is on their own.
With Erik's original proposal (webrev.02), you will, by default, get a
hotspot "server" JVM variant that is identical to what you got without
the patch. This should definitely cover your case.
You will also get all the non-hotspot libraries built as hardened. You
*can* get the JDK libraries built non-hardened, by removing the
${$2NO_SPECULATIVE_CTI_CFLAGS} from the line
$1_CFLAGS_JDK="${$1_DEFINES_CPU_JDK} ${$1_CFLAGS_CPU}
${$1_CFLAGS_CPU_JDK} ${$1_TOOLCHAIN_CFLAGS} ${$2NO_SPECULATIVE_CTI_CFLAGS}".
As I said, I believe this is enough to support that case.
I don't share your faith or confidence in the quality of any software
rushed out in a fairly short space of time. Prudence, if nothing else,
says you should be able to not build this way IMHO.
AFAIU, these compiler flags has received extensive testing inside
Oracle. It is also part of a global, high-visibility project, where key
players have had time to prepare for handling the issues ahead of the
public awareness of the exploits. *And* it's been almost half a year
since the Spectre exploit was made public.
I have much more faith in enabling these flags than I'd have for e.g.
upgrading to a newer version of Solaris Studio. :-)
In fact, I could even argue that "server" should be hardened *by
default*, and that we should instead introduce a non-hardened JVM
named something akin to "quick-but-dangerous-server" instead. But I
realize that a 25% performance hit is hard to swallow, so I won't
push this agenda.
2) It is by no means clear that "--enable-hardened-jdk" does not
harden all aspects of the JDK! If we should keep the option (which I
definitely do not think we should!) it should be renamed to
"--enable-hardened-libraries", or something like that. And it should
be on by default, so it should be a "--disabled-hardened-jdk-libraries".
Also, the general-sounding name "hardened" sounds like it might
encompass more things than it does. What if I disabled a hardened jdk
build, should I still get stack banging protection? If so, you need
to move a lot more security-related flags to this option. (And, just
to be absolutely clear: I don't think you should do that.)
3) Having two completely different ways of turning on Spectre
protection for hotspot is just utterly confusing! This was a perfect
example of how to use the JVM features, just as in the original patch.
Okay. I have had some confusion over "features" versus "variants"
based on Eriks earlier comments. Erik's email from June 6 first states:
"I agree, and you sort of can. By adding the jvm feature
"no-speculative-cti" to any jvm variant, you get the flags."
but then later said:
"We don't see the point in giving the choice on the JDK libraries ..."
by which I now think he meant not giving the choice at the VM variant
level, but I mistook it for meaning at the "feature" level. Hence I
came back with the two flags suggestion. If we can already select
features arbitrarily at configure time then this is all addressed
already.
Apologies for the confusion.
Well, now *I* am confused. ;-)
Let's separarate two components: hotspot, and the rest of the native
code (the "JDK libraries").
For hotspot, the following holds:
* You can enable or disable no-speculative-cti as a feature on the
configure command line, by "--with-jvm-features=no-speculative-cti" (to
enable) or "--with-jvm-features=-no-speculative-cti" (to disable). This
change applies to *all* JVM variants that are built; there is currently
no command-line support for enabling or disabling features on a
per-JVM-variant level. (There's no real hinderance to doing so; it's
just not yet implemented).
* Erik defines a new JVM variant, which is identical to server, but has
also no-speculative-cti enabled. This is not built by default, but can
be enabled by --with-jvm-variants=server,hardened. Oracle will build
OpenJDK this way.
If you do not want hardening, you just do not build the "hardened"
variant, and you do not add the no-speculative-cti feature.
If you want hardening all over the line (and no runtime user choice),
you add --with-jvm-feature=no-speculative-cti.
Alright?
Erik's second comment "We don't see the point in giving the choice on
the JDK libraries ..." applies not to Hotspot, but to the rest of the
native libraries (the "JDK libraries"). Here, we will just add the
Spectre mitigation flags, without a user runtime choice of using
hardened or non-hardened libraries. The reason for this is that the
hardening did not have a measurable performance impact.
The builder of the JDK still has the ability to build the JDK libraries
without the hardening flags, but that will require modifying the
configure script, just as the case is today if the builder should wish
to disable any other of all the flags we enable by default.
/Magnus
David
-----
If you want to have spectre mitigation enabled for both server and
client, by default, you would just need to run "configure
--with-jvm-variants=server,client
--with-jvm-features=no-speculative-cti", which will enable that
feature for all variants. That's not really hard *at all* for anyone
building OpenJDK. And it's way clearer what will happen, than a
--enable-hardened-hotspot.
4) If you are a downstream provider building OpenJDK and you are dead
set on not including Spectre mitigations in the JDK libraries,
despite being shown to have no negative effects, then you can do just
as any other downstream user with highly specialized requirements,
and patch the source. I have no sympathies for this; I can't stop it
but I don't think there's any reason for us to complicate the code to
support this unlikely case.
So, to recap, I think the webrev as published in
http://cr.openjdk.java.net/~erikj/8202384/webrev.02/ (with
"altserver" renamed to "hardened") is the way to go.
/Magnus
/Erik
