On Thu, 5 Feb 2026 01:07:46 GMT, Yasumasa Suenaga <[email protected]> wrote:
>> src/jdk.hotspot.agent/linux/native/libsaproc/ps_core.c line 609:
>>
>>> 607: // Otherwise copy vDSO memory in coredump to temporal memory generated
>>> by memfd_create().
>>> 608: // Returns FD for vDSO (should be closed by caller).
>>> 609: static int handle_vdso(struct ps_prochandle* ph, char* lib_name,
>>> size_t lib_name_len) {
>>
>> handle_vdso is called with: handle_vdso(ph, lib_name, sizeof(lib_name)):
>>
>> Was it meant to pass strlen(lib_name)?
>> But lib_name here is from the linker info, so that is the short name
>> "linux-vdso.so.1" ?
>> handle_vdso then mallocs something of the same size, but snprintfs something
>> much longer? We should just allocate a reasonable sized buffer to create
>> vdso_path, and maybe we don't need the lib_name_len parameter.
>>
>> Then handle_vdso checks /lib/modules... exists, and then opens it with
>> pathmap_open? So it has to exist without pathmap, but may also be in the
>> SA_ALTROOT directory.
>> Should handle_vdso also try pathmap_open() first?
>
>> Was it meant to pass strlen(lib_name)?
>
> `lib_name` is allocated with `char[BUF_SIZE]`, and is used in subsequent code
> in `read_shared_lib_info()`, thus I want to overwrite it to real path of vDSO
> binary at `handle_vdso()`. Hence I passed buffer size, not length of string.
>
>> Then handle_vdso checks /lib/modules... exists, and then opens it with
>> pathmap_open? So it has to exist without pathmap, but may also be in the
>> SA_ALTROOT directory.
> Should handle_vdso also try pathmap_open() first?
>
> Good catch! Fixed in new commit.
Ak ok yes thanks, lib_name is a local char[] (it's defined a long way back in
the method!).
Comment which might be clearer:
// Check for vDSO binary in kernel directory (/lib/modules/<version>/vdso),
rewrite the
// given lib_name string if found.
// Otherwise copy vDSO memory in coredump to temporal memory generated by
memfd_create().
// Returns FD for vDSO (should be closed by caller), or -1 on error.
>> src/jdk.hotspot.agent/linux/native/libsaproc/ps_core.c line 732:
>>
>>> 730:
>>> 731: if (lib_name[0] != '\0') { // ignore empty lib names
>>> 732: if (strcmp("linux-vdso.so.1", lib_name) == 0 ||
>>
>> Could we check this lib is at the VDSO address, so we know for sure it's the
>> one and only system vdso?
>> link_map_addr + LINK_MAP_ADDR_OFFSET == ps_prochandle->core->vdso_addr
>> Could do that instead of the name check, or in addition.
>
> I think it is better to compare with `lib_base` like this:
>
> if (lib_base == ph->core->vdso_addr) {
> lib_fd = handle_vdso(ph, lib_name, sizeof(lib_name));
> } else {
>
> However it is difficult because `lib_base` is calculated in subsequent code
> with program header in ELF (in `find_base_address`).
>
> Maybe we can use `lib_base_diff` at here because debug log says virtual
> address for vDSO is same with vaddr in `l_addr` in `struct link_map` as
> following, but I'm not sure.
>
> libsaproc DEBUG: first link map is at 0x7fa08970d2f0
> libsaproc DEBUG: replace vDSO: linux-vdso.so.1 ->
> /lib/modules/6.18.6-200.fc43.x86_64/vdso/vdso64.so
> libsaproc DEBUG: reading library
> /lib/modules/6.18.6-200.fc43.x86_64/vdso/vdso64.so @ 0x7fa0896d2000 [
> 0x7fa0896d2000 ]
> libsaproc DEBUG: overwrote with new address mapping (memsz 8192 -> 8192)
> libsaproc DEBUG: /lib/modules/6.18.6-200.fc43.x86_64/vdso/vdso64.so [0]
> 0x7fa0896d2000-0x7fa0896d4000: base = 0x7fa0896d2000, vaddr = 0x0, memsz =
> 0x1883, filesz = 0x1883
Yes I think we should compare (lib_base_diff == ph->core->vdso_addr),
I'm seeing the VDSO in /lib/modules with a load address of zero, so
lib_base_diff read from linkmap must be the real virtual address of the vdso.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/29400#discussion_r2768230483
PR Review Comment: https://git.openjdk.org/jdk/pull/29400#discussion_r2768224547
