On Mon, Mar 10, 2014 at 05:25:11PM +0800, arthur wrote:
> Description
> in dump_dmesg_structured() the out_buf size is 4096, and if the
> length is less than 4080( 4096-16 ) it won't really write out.
> Normally, after writing one or four chars to the out_buf, it will
> check the length of out_buf. But in extreme cases, 19 chars was
> written to the out_buf before checking the length. This may cause
> the stack corruption. If the length was 4079 (won't realy write out),
> and then write 19 chars to it. the out_buf will overflow.
>
> Solution
> Change 16 to 32 and always check the lenth of out_buf before move to
> next record, thus can make sure that at the beginning of a message
> we always have 32 bytes of buffer.
>
> Signed-off-by: arthur <zzou at redhat.com>
> ---
> vmcore-dmesg/vmcore-dmesg.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/vmcore-dmesg/vmcore-dmesg.c b/vmcore-dmesg/vmcore-dmesg.c
> index 0345660..0d323c1 100644
> --- a/vmcore-dmesg/vmcore-dmesg.c
> +++ b/vmcore-dmesg/vmcore-dmesg.c
> @@ -674,7 +674,7 @@ static void dump_dmesg_structured(int fd)
> else
> out_buf[len++] = c;
>
> - if (len >= OUT_BUF_SIZE - 16) {
> + if (len >= OUT_BUF_SIZE - 32) {
How do you know 32 is enough and will not overflow?
> write_to_stdout(out_buf, len);
> len = 0;
> }
> @@ -682,6 +682,11 @@ static void dump_dmesg_structured(int fd)
>
> out_buf[len++] = '\n';
>
> + if (len >= OUT_BUF_SIZE - 32) {
> + write_to_stdout(out_buf, len);
> + len = 0;
> + }
Why do we need this? After last check only one extra character "\n"
has been written?
I think that this overflow is coming from following sprintf().
len += sprintf(out_buf + len, "[%5llu.%06llu] ",
(long long unsigned int)imaxdiv_sec.quot,
(long long unsigned int)imaxdiv_usec.quot);
So we are printing 2 long long unsigned int and 3 more chars "[] ". I
Max long long unsigned int is (2^64 -1) = 18446744073709551615
Above takes 20 bytes. So total max length can be 20 + 20 + 3 = 43.
I think just leave 64bytes of buffer before we enter the next message
and that should be fine.
And we don't have to do this twice. Just do it once inside innermost
for loop.
Thanks
Vivek
