Skip to content

Optionally use orc_header to detect orc version#171

Closed
nilayvaish wants to merge 1 commit intocrash-utility:masterfrom
nilayvaish:master
Closed

Optionally use orc_header to detect orc version#171
nilayvaish wants to merge 1 commit intocrash-utility:masterfrom
nilayvaish:master

Conversation

@nilayvaish
Copy link
Copy Markdown

@nilayvaish nilayvaish commented Feb 8, 2024

In Linux kernel commit b9f174c811e3ae4ae8959dc57e6adb9990e913f4, a new .orc_header section was introduced. This section holds a version identifier for the ORC types in use.

This patch was tested by compiling crash and using it to read coredump of kernel. A temporary print statement was added to check that the if conditions specified are working fine.

#169

In Linux kernel commit b9f174c811e3ae4ae8959dc57e6adb9990e913f4, a new
.orc_header section was introduced.  This section holds a version
identifier for the ORC types in use.

This patch was tested by compiling crash and using it to read coredump
of kernel.  A temporary print statement was added to check that the
if conditions specified are working fine.
Comment thread x86_64.c
Comment on lines +6468 to +6469
if (try_get_symbol_data("orc_header", sizeof(orc_header), orc_header) &&
memcmp(orc_header, orc_hash_6_4, 20) == 0) {
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

We might want to move these lines since we have checks on lines 6413-6421 that can fail.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Move to where? Could you please be more specific. In addition the lines 6413-6421 currenty point to code which seems unrelated to this topic, could you please update it too, so we can know the failing checks:

  6413  
  6414                                          if (!errflag) {
  6415                                                  machdep->machspec->page_offset_force = value;
  6416                                                  error(NOTE, "setting PAGE_OFFSET to: 0x%lx\n\n",
  6417                                                          machdep->machspec->page_offset_force);
  6418                                                  continue;
  6419                                          }
  6420                                  }
  6421                          }

Comment thread x86_64.c
};
struct ORC_data *orc;
unsigned char orc_header[20];
const uint8_t orc_hash_6_4[20] = {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The orc_header in hex is hard coded, according to the commit log, is a 20-byte hash. Though I have the same value in my kernel, what if the hash changed in the future, do we still need to hard code those by then? Or will the hash stay unchanged all the time?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In addition, I would suggest to add code comment of how to get those hex number. E.g. I get those number on my machine by "readelf -x .orc_header vmlinux"

Comment thread x86_64.c
if (try_get_symbol_data("orc_header", sizeof(orc_header), orc_header) &&
memcmp(orc_header, orc_hash_6_4, 20) == 0) {
machdep->flags |= ORC_6_4;
} else if (orc->has_signal && !orc->has_end)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I still didn't understand why the previous if (orc->has_signal && !orc->has_end) check is not enough with the introducing of the kernel patch "x86/unwind/orc: Add ELF section with ORC version identifier"? Please add the root cause in the commit message if possible. Thanks!

@lian-bo
Copy link
Copy Markdown
Member

lian-bo commented Mar 25, 2025

Hi, Tao

Let's close this PR, feel free to post the patch to crash-utility email list if the patch gets updated.

In addition, currently there is a similar checking for the orc version in crash-utility code.

Patch review mail list: devel@lists.crash-utility.osci.io
Or
You can also subscribe it.

Thanks.

@lian-bo lian-bo closed this Mar 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants