[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-serviceability-dev
Subject: Request for Review: 7132199: sun/management/jmxremote/bootstrap/JvmstatCountersTest.java
From: daniel.daugherty () oracle ! com (Daniel D ! Daugherty)
Date: 2012-01-31 15:21:46
Message-ID: 4F28070A.9070007 () oracle ! com
[Download RAW message or body]
Summary: The .java_pid socket/door is always created in the
os::get_temp_directory() which is "/tmp" on BSD, Linux and
Solaris. On MacOS X, it can be either the "secure per-user
temporary directory" or "/tmp".
The .attach_pid file is created by the Java side
createAttachFile(). That code first tries to create the
file in the process' working directory and if that fails,
then it attempts to create the file in tmpdir.
src/solaris/classes/sun/tools/attach/LinuxVirtualMachine.java
lines 40-44: The new comment is right for .java_pid, but
not quite right for .attach_pid. See createAttachFile()
method on line 280; .attach_pid creation is first
attempted in the Java process' current working dir
and then in tmpdir.
src/solaris/classes/sun/tools/attach/SolarisVirtualMachine.java
lines 41-45: The new comment is right for .java_pid, but
not quite right for .attach_pid. See createAttachFile()
method on line 215; .attach_pid creation is first
attempted in the Java process' current working dir
and then in tmpdir.
test/ProblemList.txt
No comments.
Linux HSX gory details:
The following block of code only creates the .java_pid socket in the
directory named by os::get_temp_directory(). On Linux, that function
is hardcoded to return "/tmp".
src/os/linux/vm/attachListener_linux.cpp:
170 // Initialization - create a listener socket and bind it to a file
171
172 int LinuxAttachListener::init() {
173 char path[UNIX_PATH_MAX]; // socket file
174 char initial_path[UNIX_PATH_MAX]; // socket file during setup
175 int listener; // listener socket (file
descriptor
)
176
177 // register function to cleanup
178 ::atexit(listener_cleanup);
179
180 int n = snprintf(path, UNIX_PATH_MAX, "%s/.java_pid%d",
181 os::get_temp_directory(),
os::current_process_id());
182 if (n < (int)UNIX_PATH_MAX) {
183 n = snprintf(initial_path, UNIX_PATH_MAX, "%s.tmp", path);
184 }
185 if (n >= (int)UNIX_PATH_MAX) {
186 return -1;
187 }
This block of code checks the process' current directory for an .attach_pid
file before it checks os::get_temp_directory(). However, that check is
just to see if the attach mechanism should be started. If it does get
spoofed, there is little harm.
464 // If the file .attach_pid<pid> exists in the working directory
465 // or /tmp then this is the trigger to start the attach mechanism
466 bool AttachListener::is_init_trigger() {
467 if (init_at_startup() || is_initialized()) {
468 return false; // initialized at startup or
already ini
tialized
469 }
470 char fn[PATH_MAX+1];
471 sprintf(fn, ".attach_pid%d", os::current_process_id());
472 int ret;
473 struct stat64 st;
474 RESTARTABLE(::stat64(fn, &st), ret);
475 if (ret == -1) {
476 snprintf(fn, sizeof(fn), "%s/.attach_pid%d",
477 os::get_temp_directory(), os::current_process_id());
478 RESTARTABLE(::stat64(fn, &st), ret);
479 }
480 if (ret == 0) {
481 // simple check to avoid starting the attach mechanism when
482 // a bogus user creates the file
483 if (st.st_uid == geteuid()) {
484 init();
485 return true;
486 }
487 }
488 return false;
489 }
Solaris HSX gory details:
This code assumes both files are always in /tmp/...:
src/os/solaris/dtrace/jvm_dtrace.c
170 #define ATTACH_FILE_PATTERN "/tmp/.attach_pid%d"
171
172 /* fill-in the name of attach file name in given buffer */
173 static void fill_attach_file_name(char* path, int len, pid_t pid) {
174 memset(path, 0, len);
175 sprintf(path, ATTACH_FILE_PATTERN, pid);
176 }
177
178 #define DOOR_FILE_PATTERN "/tmp/.java_pid%d"
179
180 /* open door file for the given JVM */
181 static int open_door(pid_t pid) {
182 char path[PATH_MAX + 1];
183 int fd;
184
185 sprintf(path, DOOR_FILE_PATTERN, pid);
186 fd = file_open(path, O_RDONLY);
187 if (fd < 0) {
188 set_jvm_error(JVM_ERR_CANT_OPEN_DOOR);
189 print_debug("cannot open door file %s\n", path);
190 return -1;
191 }
192 print_debug("opened door file %s\n", path);
193 if (check_permission(path) != 0) {
194 set_jvm_error(JVM_ERR_DOOR_FILE_PERMISSION);
195 print_debug("check permission failed for %s\n", path);
196 file_close(fd);
197 fd = -1;
198 }
199 return fd;
200 }
201
202 /* create attach file for given process */
203 static int create_attach_file(pid_t pid) {
204 char path[PATH_MAX + 1];
205 int fd;
206 fill_attach_file_name(path, sizeof(path), pid);
207 fd = file_open(path, O_CREAT | O_RDWR);
208 if (fd < 0) {
209 set_jvm_error(JVM_ERR_CANT_CREATE_ATTACH_FILE);
210 print_debug("cannot create file %s\n", path);
211 } else {
212 print_debug("created attach file %s\n", path);
213 }
214 return fd;
215 }
The following block of code only creates the .java_pid door in the
directory named by os::get_temp_directory(). On Solaris, that function
is hardcoded to return "/tmp".
src/os/solaris/vm/attachListener_solaris.cpp
367 // Create the door
368 int SolarisAttachListener::create_door() {
369 char door_path[PATH_MAX+1];
370 char initial_path[PATH_MAX+1];
371 int fd, res;
372
373 // register exit function
374 ::atexit(listener_cleanup);
375
376 // create the door descriptor
377 int dd = ::door_create(enqueue_proc, NULL, 0);
378 if (dd < 0) {
379 return -1;
380 }
381
382 // create initial file to attach door descriptor
383 snprintf(door_path, sizeof(door_path), "%s/.java_pid%d",
384 os::get_temp_directory(), os::current_process_id());
385 snprintf(initial_path, sizeof(initial_path), "%s.tmp",
door_path);
386 RESTARTABLE(::creat(initial_path, S_IRUSR | S_IWUSR), fd);
387 if (fd == -1) {
388 debug_only(warning("attempt to create %s failed",
initial_path));
389 ::door_revoke(dd);
390 return -1;
391 }
392 assert(fd >= 0, "bad file descriptor");
393 RESTARTABLE(::close(fd), res);
This block of code checks the process' current directory for an .attach_pid
file before it checks os::get_temp_directory(). However, that check is
just to see if the attach mechanism should be started. If it does get
spoofed, there is little harm.
603 // If the file .attach_pid<pid> exists in the working directory
604 // or /tmp then this is the trigger to start the attach mechanism
605 bool AttachListener::is_init_trigger() {
606 if (init_at_startup() || is_initialized()) {
607 return false; // initialized at startup or
already ini
tialized
608 }
609 char fn[PATH_MAX+1];
610 sprintf(fn, ".attach_pid%d", os::current_process_id());
611 int ret;
612 struct stat64 st;
613 RESTARTABLE(::stat64(fn, &st), ret);
614 if (ret == -1) {
615 snprintf(fn, sizeof(fn), "%s/.attach_pid%d",
616 os::get_temp_directory(), os::current_process_id());
617 RESTARTABLE(::stat64(fn, &st), ret);
618 }
619 if (ret == 0) {
620 // simple check to avoid starting the attach mechanism when
621 // a bogus user creates the file
622 if (st.st_uid == geteuid()) {
623 init();
624 return true;
625 }
626 }
627 return false;
628 }
BSD/MacOS X HSX gory details:
This code assumes both files are always in /tmp/...:
src/os/bsd/dtrace/jvm_dtrace.c
170 #define ATTACH_FILE_PATTERN "/tmp/.attach_pid%d"
171
172 /* fill-in the name of attach file name in given buffer */
173 static void fill_attach_file_name(char* path, int len, pid_t pid) {
174 memset(path, 0, len);
175 sprintf(path, ATTACH_FILE_PATTERN, pid);
176 }
177
178 #define DOOR_FILE_PATTERN "/tmp/.java_pid%d"
179
180 /* open door file for the given JVM */
181 static int open_door(pid_t pid) {
182 char path[PATH_MAX + 1];
183 int fd;
184
185 sprintf(path, DOOR_FILE_PATTERN, pid);
186 fd = file_open(path, O_RDONLY);
187 if (fd < 0) {
188 set_jvm_error(JVM_ERR_CANT_OPEN_DOOR);
189 print_debug("cannot open door file %s\n", path);
190 return -1;
191 }
192 print_debug("opened door file %s\n", path);
193 if (check_permission(path) != 0) {
194 set_jvm_error(JVM_ERR_DOOR_FILE_PERMISSION);
195 print_debug("check permission failed for %s\n", path);
196 file_close(fd);
197 fd = -1;
198 }
199 return fd;
200 }
201
202 /* create attach file for given process */
203 static int create_attach_file(pid_t pid) {
204 char path[PATH_MAX + 1];
205 int fd;
206 fill_attach_file_name(path, sizeof(path), pid);
207 fd = file_open(path, O_CREAT | O_RDWR);
208 if (fd < 0) {
209 set_jvm_error(JVM_ERR_CANT_CREATE_ATTACH_FILE);
210 print_debug("cannot create file %s\n", path);
211 } else {
212 print_debug("created attach file %s\n", path);
213 }
214 return fd;
215 }
The following block of code only creates the .java_pid socket in the
directory named by os::get_temp_directory(). On BSD, that function
is hardcoded to return "/tmp". On MacOS X, that function returns
the path of the "secure per-user temporary directory" if one is
configured or "/tmp" if one is not.
src/os/bsd/vm/attachListener_bsd.cpp
170 // Initialization - create a listener socket and bind it to a file
171
172 int BsdAttachListener::init() {
173 char path[UNIX_PATH_MAX]; // socket file
174 char initial_path[UNIX_PATH_MAX]; // socket file during setup
175 int listener; // listener socket (file
descriptor
)
176
177 // register function to cleanup
178 ::atexit(listener_cleanup);
179
180 int n = snprintf(path, UNIX_PATH_MAX, "%s/.java_pid%d",
181 os::get_temp_directory(),
os::current_process_id());
182 if (n < (int)UNIX_PATH_MAX) {
183 n = snprintf(initial_path, UNIX_PATH_MAX, "%s.tmp", path);
184 }
185 if (n >= (int)UNIX_PATH_MAX) {
186 return -1;
187 }
This block of code checks the process' current directory for an .attach_pid
file before it checks os::get_temp_directory(). However, that check is
just to see if the attach mechanism should be started. If it does get
spoofed, there is little harm.
476 // If the file .attach_pid<pid> exists in the working directory
477 // or /tmp then this is the trigger to start the attach mechanism
478 bool AttachListener::is_init_trigger() {
479 if (init_at_startup() || is_initialized()) {
480 return false; // initialized at startup or
already ini
tialized
481 }
482 char path[PATH_MAX + 1];
483 int ret;
484 struct stat st;
485
486 snprintf(path, PATH_MAX + 1, "%s/.attach_pid%d",
487 os::get_temp_directory(), os::current_process_id());
488 RESTARTABLE(::stat(path, &st), ret);
489 if (ret == 0) {
490 // simple check to avoid starting the attach mechanism when
491 // a bogus user creates the file
492 if (st.st_uid == geteuid()) {
493 init();
494 return true;
495 }
496 }
497 return false;
498 }
On 1/31/12 7:32 AM, Daniel D. Daugherty wrote:
> Sorry, sent that last one as "reply to list" instead of "reply to all"...
>
> I think you need to hold up on this one. I don't think the
> assumption that .java_pid and .attach_pid files are always
> in /tmp is a good one.
>
> More in a little bit.
>
> Dan
>
> On 1/31/12 1:10 AM, Staffan Larsen wrote:
>> Moving "/tmp" to a static field, made it easier to write a comment
>> explaining the rationale as well.
>>
>> Updated webrev: http://cr.openjdk.java.net/~sla/7132199/webrev.02/
>> <http://cr.openjdk.java.net/%7Esla/7132199/webrev.02/>
>>
>> Thanks,
>> /Staffan
>>
>> On 31 jan 2012, at 01:23, David Holmes wrote:
>>
>>> On 31/01/2012 3:28 AM, Dmitry Samersoff wrote:
>>>> On 2012-01-30 16:28, Staffan Larsen wrote:
>>>>>> 2. If you decide to hardcode "/tmp" please, create a global constant
>>>>>> for it.
>>>>>
>>>>> I don't agree that this would make the code easier to read or
>>>>> maintain.
>>>>> I should, however, include a comment saying that the file is
>>>>> always in
>>>>> /tmp regardless of the value of java.io.tmpdir.
>>>
>>> Staffan: I still think changing the static field tmpdir to refer to
>>> "/tmp" is cleaner then putting "/tmp" in all the use-sites.
>>>
>>>> /tmp is common but not mandatory, especially if we speak about
>>>> embedded
>>>> systems.
>>>
>>> Dmitry: The point is that the VM will always put the file in /tmp.
>>> That's wrong but the issue here is making the management Java code
>>> match the hotspot code.
>>>
>>>> Native code should use P_tmpdir constant from stdio.h rather than
>>>> hardcode "/tmp".
>>>>
>>>> As we can't access it from java I recommend to create a global
>>>> constant
>>>> somewhere to reduce possible future porting efforts.
>>>>
>>>>
>>>>> Changing the tmpdir static would be a smaller fix, but all the cwd
>>>>> code
>>>>> would then remain. Yes, HotSpot never writes to cwd.
>>>>
>>>> I agree with Staffan, that looks for socket/door in cwd should be
>>>> removed.
>>>
>>> Ok, if it is never needed then remove it.
>>>
>>> David
>>>
>>>> -Dmitry
>>>>
>>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic