linux-kernel - Re: [PATCH next v2 1/2] printk: avoid and/or handle record truncation

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20200930112836.GC29288@alley>
Date:   Wed, 30 Sep 2020 13:28:36 +0200
From:   Petr Mladek <pmladek@...e.com>
To:     John Ogness <john.ogness@...utronix.de>
Cc:     Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
        Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Marek Szyprowski <m.szyprowski@...sung.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH next v2 1/2] printk: avoid and/or handle record truncation

On Wed 2020-09-30 12:30:50, John Ogness wrote:
> On 2020-09-30, Sergey Senozhatsky <sergey.senozhatsky@...il.com> wrote:
> > On (20/09/30 11:07), John Ogness wrote:
> >>  bool prb_reserve_in_last(struct prb_reserved_entry *e, struct printk_ringbuffer *rb,
> >> -			 struct printk_record *r, u32 caller_id);
> >> +			 struct printk_record *r, u32 caller_id, unsigned int max_size);
> >
> > Isn't `max_size' always LOG_LINE_MAX?
> 
> Yes. But I still think it makes sense that it is an argument of the
> function. It is quite an important setting and hard-coding it within the
> ringbuffer code might lead to hidden problems later.

I personally prefer the argument as well.

It is true that printk_ringbuffer is not a fully generic ringbuffer.
It has very special behavior so that it can be hardly be used
anywhere else.

Sometimes it is not clear what printk() requirements should be passed via
the API or hardcoded into the ring buffer code. IMHO, it depends on the code
complexity.

Anyway, I see hardcoded limit more like a hack. It limits something
somewhere so that some other code somewhere else is safe to use.

And printk.c is really bad from this point. It sometimes does not
check for overflow because it "knows" that the buffers are big
enough. But it is error prone code, especially when there are more
limits defined (pure text, prefix, extended prefix). And it
will be worse if we allow to add more optional information
into the prefix.

Best Regards,
Petr