Messages in this thread | | | Date | Fri, 11 Nov 2016 19:07:09 +0100 | From | Petr Mladek <> | 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@suse.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
| |