Re: [RFC V2] printk: add warning while drop partial text in msg
From: pierre kuo
Date: Thu Aug 10 2017 - 12:26:15 EST
hi Sergey
> this is not the only place that can truncate the message.
> vprintk_emit() can do so as well /* vscnprintf() */. but
> I think we don't care that much. a user likely will notice
> truncated messages. we report lost messages, because this
> is a completely different sort of problem.
Usually people will more easily find message truncated from semantics
by vscnprintf,
since it brute force truncate input message by the upper limit of output buffer.
In msg_print_text, it use memchr to find "\n" as copying unit while
memcping msg->text to output buffer.
People will more difficultly to find out some part of message is left behind.
That is the reason I try to add such warning in msg_print_text.
>
>
> [..]
>> @@ -1264,8 +1270,23 @@ static size_t msg_print_text(const struct printk_log *msg, bool syslog, char *bu
>>
>> if (buf) {
>> if (print_prefix(msg, syslog, NULL) +
>> - text_len + 1 >= size - len)
>> + text_len + 1 >= size - len) {
>> + /* below stores dropped characters
>> + * related information in next msg
>> + */
>> + size_t drop_len;
>> +
>> + drop_len = scnprintf(drop_msg,
>> + MAX_DROP_MSG_LENGTH,
>> + "<%u characters dropped>",
>> + (next) ?
>> + (unsigned int)(text_size + next - text) :
>> + (unsigned int)text_size);
>> + drop_msg[drop_len] = 0;
>> + log_store(msg->facility, msg->level, msg->flags,
>> + 0, NULL, 0, drop_msg, strlen(drop_msg));
>> break;
>> + }
>
> this change, most likely, will confuse people. because msg_print_text() is
> called on a message that is being currently processed, which is not
> necessarily the last message in the logbuf. for example, see console_unlock().
> we do something like this:
>
> while (console_seq != log_next_seq) {
> msg = log_from_idx(console_idx);
> msg_print_text(msg);
> console_idx = log_next(console_idx);
> console_seq++;
> }
>
> your log_store(), invoked from msg_print_text(), will append the error
> message to the logbuf (tail), possibly far-far-far away from console_idx.
> so your "characters dropped" warning will appear much later.
>
>
>> len += print_prefix(msg, syslog, buf + len);
>> memcpy(buf + len, text, text_len);
>
>
> but more importantly, msg_print_text() is called from several places. and
> can even be called from a user space, potentially triggering the same
> "<characters dropped>" error log_store() over and over again, wiping out
> the actually important kernel messages. which is
> a) not nice
> and
> b) can be used to deliberately "hide" something really important.
>
>
> so, no. sorry, I don't like this change.
>
> -ss