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]

Date:	Sun, 16 Feb 2014 19:19:41 -0500
From:	"Banerjee, Debabrata" <dbanerje@...mai.com>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
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 2/16/14, 6:59 PM, "Linus Torvalds" <torvalds@...ux-foundation.org>
wrote:

>On Sun, Feb 16, 2014 at 3:23 PM, Banerjee, Debabrata
><dbanerje@...mai.com> wrote:
>>
>> The explanation is: the loops look identical but they are not. When a
>> record is printed first, its size can expand due to adding the prefix
>>and
>> timestamp. The second loop is calculating len with the first line
>>printed
>> possibly changing every iteration.
>
>That still makes zero sense.
>
>The size should damn well not change, because we *should* be calling
>it with the exact same flags. If the size changes, something is wrong.

The problem is that it starts discarding records from the top of the list,
and the size of the first line is based on the previous line. When we
discard the *first* line off the list, then the size of the next line can
change since it is the new first line. My v1 patch was flawed because it
didn't consider just the first line.

>So my suspicion is that the *real* bug is that
>
>    prev = 0;
>
>before that *third* loop, because it means that the first time through
>that loop (when we did *not* reset "seq/idx" to the beginning, we use
>the wrong "prev" value.

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. seq is not reset
because the second loop seeks to the seq where the third loop starts
printing.

I think I really fixed it properly with my v2 patch, please take a look.

-Debabrata

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/