All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
To: Joe Perches <joe@perches.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Chenggang Wang <wangchenggang@vivo.com>,
	Petr Mladek <pmladek@suse.com>,
	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] printk: Move and rename maximum printk output line length defines
Date: Mon, 25 May 2020 17:38:45 +0900	[thread overview]
Message-ID: <20200525083845.GD755@jagdpanzerIV.localdomain> (raw)
In-Reply-To: <a21d75fce03991d3c8eb1f4d130572d8a04f8686.camel@perches.com>

On (20/05/21 11:40), Joe Perches wrote:
> Make the printk output maximum line length globally available.
> 
> This can be used to emit logging messages lines that exceed
> the single printk maximum length line.
> 
> e.g.: the kernel boot command on ARM can be up to 2048 chars

Speaking of the kernel boot command I still think that we can use
the existing approach here - split the command line and feed it
token by token to pr_cont(). printk() will handle everything
internally without exposing any implementation details (different
prefix sizes which depend on .config, etc. etc.) and requiring less
code.

> +/* Maximum length of a single printk */
> +
> +#ifdef CONFIG_PRINTK
> +
> +#ifdef CONFIG_PRINTK_CALLER
> +#define PRINTK_PREFIX_MAX	48
> +#else
> +#define PRINTK_PREFIX_MAX	32
> +#endif
> +#define PRINTK_LOG_LINE_MAX	(1024 - PRINTK_PREFIX_MAX)
> +
> +#else
> +
> +#define PRINTK_PREFIX_MAX	0
> +#define PRINTK_LOG_LINE_MAX	0
> +
> +#endif

!CONFIG_PRINTK case is concerning.

We depend on correct zero-sized LOG_LINE handling, otherwise things
like "p + PRINTK_LOG_LINE_MAX - 1" or "(p - line) < PRINTK_LOG_LINE_MAX - 1)"
are ticking bombs, waiting for !PRINTK config and off-by-one writes/reads.
Unless the code in question does more checks or is wrapped into
'#ifdef CONFIG_PRINTK', like you did in your print_cmdline() patch.

We may do a counter measure here, and simply undefine PRINTK_LOG_LINE_MAX
for !PRINTK config, thus forcing any code which uses PRINTK_LOG_LINE_MAX
to be wrapped into `#ifdef CONFIG_PRINTK`. But I think that pr_cont()-based
loop is safer and is easier to code.

If we set to implement "print any random very large string", then I'd
prefer to do it on the printk() side, just like was suggested by Andrew,
rather than forcing people to implement similar loops in various parts
of the kernel.

	-ss

  reply	other threads:[~2020-05-25  8:38 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-19  3:29 [PATCH] init/main.c: Print all command line when boot 王程刚
2020-05-19  3:44 ` Andrew Morton
2020-05-19  5:09   ` Joe Perches
2020-05-19 14:34     ` Arvind Sankar
2020-05-19 19:42     ` [RFC PATCH 0/2] printk/init: multi-line kernel command line logging Joe Perches
2020-05-19 19:42       ` [RFC PATCH 1/2] printk: Move and rename maximum printk output line length defines Joe Perches
2020-05-21 13:49         ` Petr Mladek
2020-05-19 19:42       ` [RFC PATCH 2/2] init: Allow multi-line output of kernel command line Joe Perches
2020-05-20  4:41         ` Sergey Senozhatsky
2020-05-20  4:58           ` Joe Perches
2020-05-20 12:10             ` Sergey Senozhatsky
2020-05-20 20:36               ` Joe Perches
2020-05-21  1:00                 ` Andrew Morton
2020-05-21  2:09                   ` Joe Perches
2020-05-21  4:36                   ` Sergey Senozhatsky
2020-05-21  4:40                     ` Andrew Morton
2020-05-21 12:31                       ` Petr Mladek
2020-05-21 15:08                         ` Arvind Sankar
2020-05-21 12:48                       ` Sergey Senozhatsky
2020-05-21  4:32                 ` Sergey Senozhatsky
2020-05-21 13:46         ` Petr Mladek
2020-05-21 15:59         ` Arvind Sankar
2020-05-21 16:09           ` Joe Perches
2020-06-03  8:48         ` [init] d0bcc26c0d: BUG:kernel_hang_in_early-boot_stage,last_printk:early_console_in_setup_code kernel test robot
2020-06-03  8:48           ` [init] d0bcc26c0d: BUG:kernel_hang_in_early-boot_stage, last_printk:early_console_in_setup_code kernel test robot
2020-05-21 18:40       ` [PATCH] printk: Move and rename maximum printk output line length defines Joe Perches
2020-05-25  8:38         ` Sergey Senozhatsky [this message]
2020-05-20 14:01 ` [PATCH] init/main.c: Print all command line when boot Masami Hiramatsu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200525083845.GD755@jagdpanzerIV.localdomain \
    --to=sergey.senozhatsky@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=joe@perches.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pmladek@suse.com \
    --cc=rostedt@goodmis.org \
    --cc=wangchenggang@vivo.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.