linux-kernel - Re: [PATCH RESEND 1/3] printk: convert byte-buffer to variable-length record buffer

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: <CAPXgP12jXOJzTTgwQkm7XK_ig2iEKAbhgNkF+MRR6_S49Tqx_A@mail.gmail.com>
Date:	Wed, 28 Nov 2012 17:22:39 +0100
From:	Kay Sievers <kay@...y.org>
To:	Michael Kerrisk <mtk.manpages@...il.com>
Cc:	Greg Kroah-Hartmann <greg@...ah.com>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Ingo Molnar <mingo@...nel.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH RESEND 1/3] printk: convert byte-buffer to variable-length
 record buffer

On Wed, Nov 28, 2012 at 2:33 PM, Michael Kerrisk <mtk.manpages@...il.com> wrote:
> On Thu, May 3, 2012 at 2:29 AM, Kay Sievers <kay@...y.org> wrote:
>> From: Kay Sievers <kay@...y.org>
> [...]
>>         case SYSLOG_ACTION_SIZE_UNREAD:
>> -               error = log_end - log_start;
>> +               raw_spin_lock_irq(&logbuf_lock);
>> +               if (syslog_seq < log_first_seq) {
>> +                       /* messages are gone, move to first one */
>> +                       syslog_seq = log_first_seq;
>> +                       syslog_idx = log_first_idx;
>> +               }
>> +               if (from_file) {
>> +                       /*
>> +                        * Short-cut for poll(/"proc/kmsg") which simply checks
>> +                        * for pending data, not the size; return the count of
>> +                        * records, not the length.
>> +                        */
>> +                       error = log_next_idx - syslog_idx;
>> +               } else {
>> +                       u64 seq;
>> +                       u32 idx;
>> +
>> +                       error = 0;
>> +                       seq = syslog_seq;
>> +                       idx = syslog_idx;
>> +                       while (seq < log_next_seq) {
>> +                               error += syslog_print_line(idx, NULL, 0);
>> +                               idx = log_next(idx);
>> +                               seq++;
>> +                       }
>> +               }
>> +               raw_spin_unlock_irq(&logbuf_lock);
>>                 break;
> [...]
>
> It looks as though the changes here have broken SYSLOG_ACTION_SIZE_UNREAD.

Any specifics that it causes actual problems we need to address?

> On a 2.6.31 system, immediately after SYSLOG_ACTION_READ_CLEAR, a
> SYSLOG_ACTION_SIZE_UNREAD returns 0.

Hmm, sounds like the right thing to do.

We have read everything, even cleared the buffer for later queries. So
there is nothing to read anymore for later calls, and they will
actually never return anything if they are called, so returning 0 here
sounds like the right thing. The current SYSLOG_ACTION_SIZE_UNREAD
seems to match properly the expectations one can make for
SYSLOG_ACTION_READ_ALL.

> On 3.5, immediately after SYSLOG_ACTION_READ_CLEAR, the value returned
> by SYSLOG_ACTION_SIZE_UNREAD is unchanged
>
> (i.e., assuming that the
> value returned was non-zero before SYSLOG_ACTION_SIZE_UNREAD, it is
> still nonzero afterward), even though a subsequent
> SYSLOG_ACTION_READ_CLEAR indicates that there are zero bytes to read.

Which sounds at least like weird behaviour, if not "broken".

Any indication that we need to restore the old behaviour to fix some
weird assumptions? To me the current one sounds like the better and
more correct option, and what one would expect from it. But maybe we
cannot get away with it ...

(I hope I understood what you explained correctly, I'm a bit confused
by the issue.)


Thanks,
Kay
--
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/