Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Date:   Mon, 25 May 2020 17:38:45 +0900
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
Message-ID: <20200525083845.GD755@jagdpanzerIV.localdomain>
References: <cover.1589916689.git.joe@perches.com>
 <a21d75fce03991d3c8eb1f4d130572d8a04f8686.camel@perches.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <a21d75fce03991d3c8eb1f4d130572d8a04f8686.camel@perches.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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