Re: LINE_MAX: was: Re: [PATCH printk-rework 04/12] printk: define CONSOLE_LOG_MAX in printk.h

[John Ogness]

On 2021-01-29, Petr Mladek <pmladek@xxxxxxxx> wrote:
>> diff --git a/include/linux/printk.h b/include/linux/printk.h
>> index fe7eb2351610..6d8f844bfdff 100644
>> --- a/include/linux/printk.h
>> +++ b/include/linux/printk.h
>> @@ -45,6 +45,7 @@ static inline const char *printk_skip_headers(const char *buffer)
>>  }
>>  
>>  #define CONSOLE_EXT_LOG_MAX	8192
>> +#define CONSOLE_LOG_MAX		1024
>>  
>>  /* printk's without a loglevel use this.. */
>>  #define MESSAGE_LOGLEVEL_DEFAULT CONFIG_MESSAGE_LOGLEVEL_DEFAULT
>> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
>> index ec2174882b8e..5faf9c0db171 100644
>> --- a/kernel/printk/printk.c
>> +++ b/kernel/printk/printk.c
>> @@ -410,7 +410,7 @@ static u64 clear_seq;
>>  #else
>>  #define PREFIX_MAX		32
>>  #endif
>> -#define LOG_LINE_MAX		(1024 - PREFIX_MAX)
>> +#define LOG_LINE_MAX		(CONSOLE_LOG_MAX - PREFIX_MAX)
>
> CONSOLE_LOG_MAX defines size of buffers that are written by
> record_print_text(). We must make sure that all stored
> messages can actually get printed even with the trailing '\0'.
>
> We should limit the stored messages by:
>
> /*
>  * Console log buffer needs extra space for the trailing '\0',
>  * see reccord_print_text().
>  */
> #define LOG_LINE_MAX		(CONSOLE_LOG_MAX - PREFIX_MAX - 1)
>
> It should not be a big problem. The PREFIX_MAX size has already
> increased in the patch, for example, because of the caller ID.
>
> Does it make sense, please?

If we want to make sure "all stored messages can actually get printed",
then CONSOLE_LOG_MAX needs to be set to:

   PREFIX_MAX * LOG_LINE_MAX + 1

and we should be specifying LOG_LINE_MAX instead of
CONSOLE_LOG_MAX. record_print_text() adds up to PREFIX_MAX for every
'\n' in the message.

I was initially confused by this, which led to my patch [0] to fix
it. But then I realized that the buffer is way too small anyway. If we
want to fix the issue, then LOG_LINE_MAX needs to be much larger.

IMO it makes no sense to do the -1 change because the buffer is too
small anyway.

John Ogness

[0] https://lkml.kernel.org/r/20210120194106.26441-2-john.ogness@xxxxxxxxxxxxx