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

[Dave Anderson]

----- Original Message -----
> (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

Queued for crash-6.0.5.

Thanks,
  Dave