Looks good to me.
/Jesper
> On 11 Jun 2018, at 22:42, Erik Joelsson <erik.joels...@oracle.com> wrote:
>
> Hello,
>
> Based on the discussion here, I have reverted back to something more similar
> to webrev.02, but with a few changes. Mainly fixing a bug that caused
> JVM_FEATURES_hardened to not actually be the same as for server (if you have
> custom additions in configure). I also added a check so that configure fails
> if you try to enable either variant hardened or feature no-speculative-cti
> and the flags aren't available.
>
> Webrev: http://cr.openjdk.java.net/~erikj/8202384/webrev.05/index.html
>
> /Erik
>
> On 2018-06-11 00:10, 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!
>>
>> 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.
>>
>> 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
>>
>
