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
| ||
|
Date: Fri, 11 Nov 2016 19:07:09 +0100 From: Petr Mladek <pmladek@...e.com> To: Steven Rostedt <rostedt@...dmis.org> Cc: Linus Torvalds <torvalds@...ux-foundation.org>, Joe Perches <joe@...ches.com>, Andrew Morton <akpm@...ux-foundation.org>, Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>, Jason Wessel <jason.wessel@...driver.com>, Jaroslav Kysela <perex@...ex.cz>, Takashi Iwai <tiwai@...e.com>, Chris Mason <clm@...com>, Josef Bacik <jbacik@...com>, David Sterba <dsterba@...e.com>, linux-kernel@...r.kernel.org Subject: Re: [PATCH v2 1/4] printk/NMI: Handle continuous lines and missing newline On Fri 2016-11-11 12:28:51, Steven Rostedt wrote: > On Wed, 9 Nov 2016 13:41:28 +0100 > Petr Mladek <pmladek@...e.com> wrote: > > > > /* > > @@ -135,8 +170,8 @@ static void __printk_nmi_flush(struct irq_work *work) > > __RAW_SPIN_LOCK_INITIALIZER(read_lock); > > struct nmi_seq_buf *s = container_of(work, struct nmi_seq_buf, work); > > unsigned long flags; > > - size_t len, size; > > - int i, last_i; > > + size_t len; > > + int i; > > > > /* > > * The lock has two functions. First, one reader has to flush all > > @@ -154,12 +189,14 @@ static void __printk_nmi_flush(struct irq_work *work) > > /* > > * This is just a paranoid check that nobody has manipulated > > * the buffer an unexpected way. If we printed something then > > - * @len must only increase. > > + * @len must only increase. Also it should never overflow the > > + * buffer size. > > */ > > - if (i && i >= len) { > > + if ((i && i >= len) || len > sizeof(s->buffer)) { > > What's wrong with using s->len? Isn't that what is inside the buffer? > Couldn't just checking against the buffer size print garbage? Note that this is not the classic seq_buf. It is struct nmi_seq_buf where "len" is atomic_t and buffer is defined as buffer[NMI_LOG_BUF_LEN]. It is just a paranoid check that should newer be true if the implementation is correct. I believe that it makes sense as is. Best Regards, Petr
Powered by blists - more mailing lists