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

[Dave Anderson]

----- 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.

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.

So I'm just re-writing/simplifying the read_string() function to
zero out and then try to read maxlen bytes into the user buffer, then 
zero out everything after the first NULL that it finds, and return
the number of consecutive non-NULL characters from the starting
address.

Thanks,
  Dave