linux-kernel - Re: [PATCH] printk: Do not lose last line in kmsg dump

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190709101230.GA16909@jagdpanzerIV>
Date:   Tue, 9 Jul 2019 19:12:30 +0900
From:   Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
To:     Vincent Whitchurch <vincent.whitchurch@...s.com>
Cc:     pmladek@...e.com, sergey.senozhatsky@...il.com,
        rostedt@...dmis.org, linux-kernel@...r.kernel.org,
        Vincent Whitchurch <rabinv@...s.com>
Subject: Re: [PATCH] printk: Do not lose last line in kmsg dump

On (07/09/19 10:10), Vincent Whitchurch wrote:
> A dump of a 64-byte buffer filled by kmsg_dump_get_buffer(), before this
> patch:
> 
>  00000000: 3c 30 3e 5b 20 20 20 20 36 2e 35 32 32 31 39 37  <0>[    6.522197
>  00000010: 5d 20 41 41 41 41 41 41 41 41 41 41 41 41 41 0a  ] AAAAAAAAAAAAA.
>  00000020: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>  00000030: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> 
> After this patch:
> 
>  00000000: 3c 30 3e 5b 20 20 20 20 36 2e 34 32 37 35 30 32  <0>[    6.427502
>  00000010: 5d 20 41 41 41 41 41 41 41 41 41 41 41 41 41 0a  ] AAAAAAAAAAAAA.
>  00000020: 3c 30 3e 5b 20 20 20 20 36 2e 34 32 37 37 36 39  <0>[    6.427769
>  00000030: 5d 20 42 42 42 42 42 42 42 42 31 32 33 34 35 0a  ] BBBBBBBB12345.

[..]

> @@ -1318,7 +1318,7 @@ static size_t msg_print_text(const struct printk_log *msg, bool syslog,
>  		}
>  
>  		if (buf) {
> -			if (prefix_len + text_len + 1 >= size - len)
> +			if (prefix_len + text_len + 1 > size - len)
>  				break;

So with this patch the last byte of the buffer is 0xA. It's a bit
uncomfortable that `len', which we return from msg_print_text(),
now points one byte beyond the buffer:

	buf[len++] = '\n';
	return len;

This is not very common. Not sure what usually happens to kmsg_dump
buffers, but anyone who'd do a rather innocent

	kmsg_dump(buf, &len);
	buf[len] = 0x00;

will write to something which is not a kmsg buffer (in some cases).

	-ss