On Sat 2023-01-14 19:14:29, Sergey Senozhatsky wrote: > On (23/01/13 15:46), coverity-bot wrote: > > *** CID 1530570: Memory - corruptions (OVERRUN) > > kernel/printk/printk.c:2738 in console_prepend_dropped() > > 2732 /* Truncate the message, but keep it terminated. */ > > 2733 pmsg->outbuf_len = outbuf_sz - (len + 1); > > 2734 outbuf[pmsg->outbuf_len] = 0; > > 2735 } > > 2736 > > 2737 memmove(outbuf + len, outbuf, pmsg->outbuf_len + 1); > > vvv CID 1530570: Memory - corruptions (OVERRUN) > > vvv Overrunning buffer pointed to by "scratchbuf" of 1024 bytes by passing it to a function which accesses it at byte offset 1998 using argument "len" (which evaluates to 1999). [Note: The source code implementation of the function has been overridden by a builtin model.] > > 2738 memcpy(outbuf, scratchbuf, len); > > 2739 pmsg->outbuf_len += len; > > 2740 } > > 2741 #else > > 2742 #define console_prepend_dropped(pmsg, dropped) > > 2743 #endif /* CONFIG_PRINTK */ > [..] > > Human notes from Kees: > > > > I'm not sure how it got 1998, but I do see that snprintf() should > > probably be scnprintf(), otherwise "len" might be a lie (i.e. it'll hold > > what it WANTED to write, rather than what it actually wrote). > > Cannot imagine how "** %lu printk messages dropped **\n" can expand into > 1998 bytes. Does coverity have a "verbose" mode? I guess that coverity tries to pass some random string that is longer than the provided buffer. The code might be safe with the current size of the buffer and the string. But it is true that the following is wrong: len = snprintf(scratchbuf, scratchbuf_sz, "** %lu printk messages dropped **\n", dropped); As Kees pointed out in the human comment, we should use scnprintf() that will return the really written length of the string that fits into the buffer. I am going to send a patch. Best Regards, Petr
