linux-kernel - Re: [PATCH] printk: Fix discarding of records

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: <CA+55aFyywOm7pWrX4=04LTEJ6xYK4xDTxGRqes-MOT212PnWPw@mail.gmail.com>
Date:	Sun, 16 Feb 2014 16:41:06 -0800
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	"Banerjee, Debabrata" <dbanerje@...mai.com>
Cc:	Kay Sievers <kay.sievers@...y.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Jeff Mahoney <jeffm@...e.com>,
	"dbavatar@...il.com" <dbavatar@...il.com>,
	"Hunt, Joshua" <johunt@...mai.com>, stable <stable@...r.kernel.org>
Subject: Re: [PATCH] printk: Fix discarding of records

On Sun, Feb 16, 2014 at 4:19 PM, Banerjee, Debabrata
<dbanerje@...mai.com> wrote:
>
> No that can't be right, the prev value after every loop is the msg->flags
> from the *last* line in the list, which has no relation to the *first*, so
> reusing it for the top of the next loop is nonsense.

Please, Debabrata, humor me, and just try the patch.

And try reading the source code. Because your statement is BS.

The third loop does *not* start again from the first line! It
*continues* from where the second loop ended. Which is exactly why
clearing "prev" is *wrong*. Because the *last* line that the second
loop processes is indeed the previous line before the *first* line
that the third loop starts processing.

No, I haven't tested my patch, and maybe it's broken for some subtle
reason I'm missing too. But neither of your patches really make sense,
although I can believe that your second patch happens to get the
buffer size right almost by accident. Why? It's by virtue of the
"prefix" for a line generally being the same length, so when you
discount the prefix of the last line that you *don't* print, you by
accident get as much room as the - extraneous - prefix of the *next*
line that then actually gets copied. See?

(Btw, just to clarify: kmsg_dump_get_buffer() has the exact same logic
and the exact same bug, so as with your patches, you should remove the
"prev = 0" from before the third loop from that function too. Because
exactly as with syslog_print_all(), the third loop *continues* where
the second loop stops, and thus clearing "prev" is actually wrong).

That extended patch attached, now without whitespace damage. But still
completely and utterly untested.

                         Linus

View attachment "patch.diff" of type "text/plain" (687 bytes)