Skip to content

Commit

Permalink
8243936: NonWriteable system properties are actually writeable
Browse files Browse the repository at this point in the history
Backport-of: 686ca5a
  • Loading branch information
RealLucy committed May 15, 2023
1 parent 507a392 commit 3139666
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 20 deletions.
45 changes: 27 additions & 18 deletions src/hotspot/share/runtime/arguments.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2065,7 +2065,7 @@ bool Arguments::check_vm_args_consistency() {
if (status && EnableJVMCI) {
PropertyList_unique_add(&_system_properties, "jdk.internal.vm.ci.enabled", "true",
AddProperty, UnwriteableProperty, InternalProperty);
if (!create_numbered_property("jdk.module.addmods", "jdk.internal.vm.ci", addmods_count++)) {
if (!create_numbered_module_property("jdk.module.addmods", "jdk.internal.vm.ci", addmods_count++)) {
return false;
}
}
Expand Down Expand Up @@ -2139,20 +2139,28 @@ bool Arguments::parse_uintx(const char* value,
return false;
}

bool Arguments::create_property(const char* prop_name, const char* prop_value, PropertyInternal internal) {
bool Arguments::create_module_property(const char* prop_name, const char* prop_value, PropertyInternal internal) {
assert(is_internal_module_property(prop_name) ||
strcmp(prop_name, "jdk.module.illegalAccess") == 0, "unknown module property: '%s'", prop_name);
size_t prop_len = strlen(prop_name) + strlen(prop_value) + 2;
char* property = AllocateHeap(prop_len, mtArguments);
int ret = jio_snprintf(property, prop_len, "%s=%s", prop_name, prop_value);
if (ret < 0 || ret >= (int)prop_len) {
FreeHeap(property);
return false;
}
bool added = add_property(property, UnwriteableProperty, internal);
// These are not strictly writeable properties as they cannot be set via -Dprop=val. But that
// is enforced by checking is_internal_module_property(). We need the property to be writeable so
// that multiple occurrences of the associated flag just causes the existing property value to be
// replaced ("last option wins"). Otherwise we would need to keep track of the flags and only convert
// to a property after we have finished flag processing.
bool added = add_property(property, WriteableProperty, internal);
FreeHeap(property);
return added;
}

bool Arguments::create_numbered_property(const char* prop_base_name, const char* prop_value, unsigned int count) {
bool Arguments::create_numbered_module_property(const char* prop_base_name, const char* prop_value, unsigned int count) {
assert(is_internal_module_property(prop_base_name), "unknown module property: '%s'", prop_base_name);
const unsigned int props_count_limit = 1000;
const int max_digits = 3;
const int extra_symbols_count = 3; // includes '.', '=', '\0'
Expand Down Expand Up @@ -2311,7 +2319,7 @@ int Arguments::process_patch_mod_option(const char* patch_mod_tail, bool* patch_
// The path piece begins one past the module_equal sign
add_patch_mod_prefix(module_name, module_equal + 1, patch_mod_javabase);
FREE_C_HEAP_ARRAY(char, module_name);
if (!create_numbered_property("jdk.module.patch", patch_mod_tail, patch_mod_count++)) {
if (!create_numbered_module_property("jdk.module.patch", patch_mod_tail, patch_mod_count++)) {
return JNI_ENOMEM;
}
} else {
Expand Down Expand Up @@ -2458,31 +2466,31 @@ jint Arguments::parse_each_vm_init_arg(const JavaVMInitArgs* args, bool* patch_m
add_init_library(name, options);
}
} else if (match_option(option, "--add-reads=", &tail)) {
if (!create_numbered_property("jdk.module.addreads", tail, addreads_count++)) {
if (!create_numbered_module_property("jdk.module.addreads", tail, addreads_count++)) {
return JNI_ENOMEM;
}
} else if (match_option(option, "--add-exports=", &tail)) {
if (!create_numbered_property("jdk.module.addexports", tail, addexports_count++)) {
if (!create_numbered_module_property("jdk.module.addexports", tail, addexports_count++)) {
return JNI_ENOMEM;
}
} else if (match_option(option, "--add-opens=", &tail)) {
if (!create_numbered_property("jdk.module.addopens", tail, addopens_count++)) {
if (!create_numbered_module_property("jdk.module.addopens", tail, addopens_count++)) {
return JNI_ENOMEM;
}
} else if (match_option(option, "--add-modules=", &tail)) {
if (!create_numbered_property("jdk.module.addmods", tail, addmods_count++)) {
if (!create_numbered_module_property("jdk.module.addmods", tail, addmods_count++)) {
return JNI_ENOMEM;
}
} else if (match_option(option, "--limit-modules=", &tail)) {
if (!create_property("jdk.module.limitmods", tail, InternalProperty)) {
if (!create_module_property("jdk.module.limitmods", tail, InternalProperty)) {
return JNI_ENOMEM;
}
} else if (match_option(option, "--module-path=", &tail)) {
if (!create_property("jdk.module.path", tail, ExternalProperty)) {
if (!create_module_property("jdk.module.path", tail, ExternalProperty)) {
return JNI_ENOMEM;
}
} else if (match_option(option, "--upgrade-module-path=", &tail)) {
if (!create_property("jdk.module.upgrade.path", tail, ExternalProperty)) {
if (!create_module_property("jdk.module.upgrade.path", tail, ExternalProperty)) {
return JNI_ENOMEM;
}
} else if (match_option(option, "--patch-module=", &tail)) {
Expand All @@ -2492,7 +2500,7 @@ jint Arguments::parse_each_vm_init_arg(const JavaVMInitArgs* args, bool* patch_m
return res;
}
} else if (match_option(option, "--illegal-access=", &tail)) {
if (!create_property("jdk.module.illegalAccess", tail, ExternalProperty)) {
if (!create_module_property("jdk.module.illegalAccess", tail, ExternalProperty)) {
return JNI_ENOMEM;
}
// -agentlib and -agentpath
Expand Down Expand Up @@ -2536,7 +2544,7 @@ jint Arguments::parse_each_vm_init_arg(const JavaVMInitArgs* args, bool* patch_m
jio_snprintf(options, length, "%s", tail);
add_instrument_agent("instrument", options, false);
// java agents need module java.instrument
if (!create_numbered_property("jdk.module.addmods", "java.instrument", addmods_count++)) {
if (!create_numbered_module_property("jdk.module.addmods", "java.instrument", addmods_count++)) {
return JNI_ENOMEM;
}
}
Expand Down Expand Up @@ -2745,7 +2753,7 @@ jint Arguments::parse_each_vm_init_arg(const JavaVMInitArgs* args, bool* patch_m
return JNI_EINVAL;
}
// management agent in module jdk.management.agent
if (!create_numbered_property("jdk.module.addmods", "jdk.management.agent", addmods_count++)) {
if (!create_numbered_module_property("jdk.module.addmods", "jdk.management.agent", addmods_count++)) {
return JNI_ENOMEM;
}
#else
Expand Down Expand Up @@ -4199,14 +4207,15 @@ void Arguments::PropertyList_unique_add(SystemProperty** plist, const char* k, c
if (plist == NULL)
return;

// If property key exist then update with new value.
// If property key exists and is writeable, then update with new value.
// Trying to update a non-writeable property is silently ignored.
SystemProperty* prop;
for (prop = *plist; prop != NULL; prop = prop->next()) {
if (strcmp(k, prop->key()) == 0) {
if (append == AppendProperty) {
prop->append_value(v);
prop->append_writeable_value(v);
} else {
prop->set_value(v);
prop->set_writeable_value(v);
}
return;
}
Expand Down
12 changes: 10 additions & 2 deletions src/hotspot/share/runtime/arguments.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,11 @@ class SystemProperty : public PathString {
}
return false;
}
void append_writeable_value(const char *value) {
if (writeable()) {
append_value(value);
}
}

// Constructor
SystemProperty(const char* key, const char* value, bool writeable, bool internal = false);
Expand Down Expand Up @@ -396,8 +401,11 @@ class Arguments : AllStatic {
static bool add_property(const char* prop, PropertyWriteable writeable=WriteableProperty,
PropertyInternal internal=ExternalProperty);

static bool create_property(const char* prop_name, const char* prop_value, PropertyInternal internal);
static bool create_numbered_property(const char* prop_base_name, const char* prop_value, unsigned int count);
// Used for module system related properties: converted from command-line flags.
// Basic properties are writeable as they operate as "last one wins" and will get overwritten.
// Numbered properties are never writeable, and always internal.
static bool create_module_property(const char* prop_name, const char* prop_value, PropertyInternal internal);
static bool create_numbered_module_property(const char* prop_base_name, const char* prop_value, unsigned int count);

static int process_patch_mod_option(const char* patch_mod_tail, bool* patch_mod_javabase);

Expand Down
40 changes: 40 additions & 0 deletions test/hotspot/jtreg/runtime/NonWriteableProperty.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*
* Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*
*/

/*
* @test
* @bug 8243936
* @summary Ensure non-writeable system properties are not writeable
*
* @run main/othervm -Djava.vm.name=Unexpected NonWriteableProperty java.vm.name Unexpected
*/

public class NonWriteableProperty {
public static void main(String[] args) {
if (System.getProperty(args[0]).equals(args[1])) {
throw new RuntimeException("Non-writeable system property " +
args[0] + " was rewritten");
}
}
}

1 comment on commit 3139666

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.