Re: [Crash-utility] Are all linux_banner type cases covered in verify_version()?

On 2023/08/18 3:08, David Mair wrote: > Hi All, > > Before I consider starting work on a patch for this I'd appreciate more > input. > > I am seeing random cases of crash failing to load reporting an x86_64 > coredump reporting a "bad" linux_banner. However, the value displayed as > the banner is: > > 0x65762078756e694c > > which is plainly ASCII text as a 64-bit number and is the little-endian > reversal of the text "Linux ver". > > It's randomly found with specific coredumps and reproduces all times > with that coredump and a given version of crash, though sometimes it > will appear when using a given coredump with one version of crash but > not with another version of crash. What I'm trying to get working is > crash current and the rest of this is experience using crash 8.0.3 only. > > I used gdb crash to debug it through verify_version(). If I breakpoint > there with gdb crash and step through the function I find that in the > section: > > >     if (!(sp = symbol_search("linux_banner"))) >         error(FATAL, "linux_banner symbol does not exist?\n"); >     else if ((sp->type == 'R') || (sp->type == 'r') || >         (THIS_KERNEL_VERSION >= LINUX(2,6,11) && (sp->type == 'D' || > sp->type == 'd')) || >          (machine_type("ARM") && sp->type == 'T') || >          (machine_type("ARM64"))) >         linux_banner = symbol_value("linux_banner"); >     else >         get_symbol_data("linux_banner", sizeof(ulong), &linux_banner); > > > * The if block is not executed, i.e. symbol_search("linux_banner") > succeeded and we have a usable struct syment for "linux_banner" in sp > * The else if block is not executed, all conditions are met or not > relevant except for the the value of sp->type in the case of > THIS_KERNEL_VERSION >= LINUX(2,6,11). But sp->type is 'B', bss segment > * The final else block is executed, we copy sizeof(ulong) bytes of > symbol data from what "linux_banner" refers to into the crash internal > linux_banner variable > > Here's how sp looks at the else if statement in the above code: > > gdb) print *sp > $2 = {value = 18446744071587233984, name = 0x5555566a735b "linux_banner", >   val_hash_next = 0x7fffe51e4338, name_hash_next = 0x7fffe51f8d38, >   type = 66 'B', cnt = 1 '\001', flags = 0 '\000', pad2 = 0 '\000'} > > ...and sp->value in hex is: > > (gdb) p/x sp->value > $5 = 0xffffffff818000c0 > > Starting crash in --minimal mode with the same core, kernel and > debuginfo so that I can try to read 0xffffffff818000c0 I find: > > crash> rd 0xffffffff818000c0 18 > ffffffff818000c0:  65762078756e694c 2e34206e6f697372   Linux version 4. > ffffffff818000d0:  34392d3038312e34 6665642d3533312e   4.180-94.135-def > ffffffff818000e0:  65672820746c7561 6c697562406f6b65   ault (geeko@buil > ffffffff818000f0:  28202974736f6864 7372657620636367   dhost) (gcc vers > ffffffff81800100:  2e382e34206e6f69 2045535553282035   ion 4.8.5 (SUSE > ffffffff81800110:  29202978756e694c 20504d5320312320   Linux) ) #1 SMP > ffffffff81800120:  20766f4e206e6f4d 35353a3930203631   Mon Nov 16 09:55 > ffffffff81800130:  204354552037353a 3033282030323032   :57 UTC 2020 (30 > ffffffff81800140:  000a293039393633 0000000000000000   36990).......... > > IOW, it is the linux_banner, it's at 0xffffffff818000c0 and if the first > sizeof(ulong) bytes are read into crash's linux_banner variable via > get_symbol_data() it makes a 64-bit number with little-endian revesral > from the string bytes "Linux ver" from the actual linux_banner text, the > same value seen in the error report when crash fails. > > If I repeat the above debug of verify_version() in gdb and at the else > if block in the above C in verify_version() I set var the sp->type to be > 'D' or 'd' then crash's linux_banner is set to 0xffffffff818000c0 > (sp->value) and the else if block is executed then the remainder of the > function successfully finds a linux_banner and gets the version > information from it and crash loads. > > In symbol_search() is the value of type in the returned struct syment a > property generated by crash (I thought not and it was based on the > kernel compilation, possible kdummp/makedumpfile but not controlled by > crash)? If I'm correct then is it safe to expect a struct syment with > type 'B' (also 'b' I believe) for linux_banner? Or is there anything > special about a bss segment type that makes it not possible to assume we > can take sp->value as-is for the address of the linux_banner string > details, i.e. we can't safely use symbol_value("linux_banner") to set > the crash linux_banner variable if the struct syment type for > linux_banner is 'B' or 'b'? > > Any comments? I'll write a patch if that's the right direction but I > need a better understanding of why symbols with the type bss segment > aren't already assumed valid sources of a linux_banner address value, > only 'D' and 'd' types are and if there is anything special I don't > understand about 'B' and 'b' types. > It seems that sp->type is set to a value from bfd_get_symbol_info() in store_symbols(). So it's from the vmlinux through the embedded gdb. Anyway, I don't think that the current check gets the point, i.e. we can check the type of the linux_banner symbol directly like this? switch (get_symbol_type("linux_banner", NULL, NULL)) { case TYPE_CODE_ARRAY: linux_banner = sp->value; break; case TYPE_CODE_PTR: get_symbol_data("linux_banner", sizeof(ulong), &linux_banner); break; default: error(WARNING, "linux_banner is unknown type\n"); linux_banner = 0; break; } I don't know why it checks the sp->type, but what it should do originally might be the above, I think. Lianbo, do you have an old vmcore that has "const char *linux_banner"? According to crash commit fce91bec5bef, 2.6.10 and older kernels have it. I'd like to test the above with it.