Re: [Crash-utility] [PATCH] avoid read_string() for no terminated buf.

(2012/03/22 4:18), Dave Anderson wrote: > > > ----- Original Message ----- >> >> >> ----- Original Message ----- >>> Hi Dave, >>> >>> I met stack smashing detection by glibc at read_string() >>> then this patch is proposal. >>> >>> *** stack smashing detected ***: crash terminated >>> ======= Backtrace: ========= >>> /lib/libc.so.6(__fortify_fail+0x4c)[0xfe12380] >>> /lib/libc.so.6(__fortify_fail+0x0)[0xfe12334] >>> ./crash[0x10147bf0] >>> ./crash(display_sys_stats+0xcf8)[0x1011cd74] >>> ./crash(main_loop+0x300)[0x10068960] >>> ./crash(current_interp_command_loop+0x48)[0x1021ac2c] >>> ./crash[0x1021bcc4] >>> ./crash(catch_errors+0x84)[0x1021a0c4] >>> ./crash[0x1021d37c] >>> ./crash(catch_errors+0x84)[0x1021a0c4] >>> ./crash(gdb_main+0x58)[0x1021d3e8] >>> ./crash(gdb_main_entry+0x6c)[0x1021d490] >>> ./crash(gdb_main_loop+0x3b4)[0x10130e5c] >>> ./crash(main+0x38c0)[0x10068650] >>> /lib/libc.so.6(+0x1f568)[0xfd36568] >>> /lib/libc.so.6(+0x1f728)[0xfd36728] >>> >>> An failed vmalloc() including non terminated with NULLCHAR is >>> root cause, >>> but I think it is better to keep other utilities without killed. >> >> This patch changes the return value of read_string() in a >> situation where the requested number of bytes does not include >> a NULL terminator. Note that the function is described like >> this: >> >> /* >> * Try to read a string of non-NULL characters from a memory >> location, >> * returning the number of characters read. >> */ >> int >> read_string(ulong kvaddr, char *buf, int maxlen) >> { >> >> The "maxlen" parameter is there to handle case where the requested >> memory read does not contain a NULL character. And there may be >> other callers that use the function to read until a NULL *or* until >> the maxlen is reached. >> >> That being said, there may be a bug in there somewhere, or it >> could be written differently, but I don't want to change the >> function's behavior (return value). >> >> You mention: >> >>> an failed vmalloc() including non terminated with NULLCHAR >>> is the root cause". >> >> Can you elaborate on what you mean by that? I want to be able >> to reproduce this, but I cannot. > > Hi Toshi, > > I'm still not clear on how you were able to make this happen in > the "normal" course of events, but I was able to kludge up a test > with more than a page-size of non-NULL characters, and did manage > to force a segmentation violation. Hi Dave, Thanks for your support and I'm sorry that my previous analysis was very poor. > There's really no reason for read_string() to buffer the data given > that it has aways zeroed out the user buffer first. And there's also > no reason for it to break up the request into per-page segments. Yes, you're right and I've been mistaken about root cause. (By skipping read_string(), true root cause was also skipped...) I added debug messages in ppc_processor_speed() and found out a direct tie to "*** stack smashing detected ***". It's a ppc specific problem, and fixed with attached patch. I'm not sure about strcasecmp() implementation or specification but if "buflen - 1" size characters are passed, looked to be detected as stack corruption. Thanks a lots for your support, Toshi