linux-kernel - Re: [PATCH v3 02/12] binfmt_flat: convert printk invocations to their modern form

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1468989000.1900.94.camel@perches.com>
Date:	Tue, 19 Jul 2016 21:30:00 -0700
From:	Joe Perches <joe@...ches.com>
To:	Nicolas Pitre <nicolas.pitre@...aro.org>,
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Cc:	Alexander Viro <viro@...iv.linux.org.uk>,
	David Howells <dhowells@...hat.com>,
	Greg Ungerer <gerg@...ux-m68k.org>
Subject: Re: [PATCH v3 02/12] binfmt_flat: convert printk invocations to
 their modern form

On Wed, 2016-07-20 at 00:20 -0400, Nicolas Pitre wrote:
> diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
[]
> @@ -15,6 +15,8 @@
>   *	JAN/99 -- coded full program relocation (gerg@...pgear.com)
>   */
>  
> +#define pr_fmt(fmt)	"BINFMT_FLAT: : " fmt

Why the double colon?
Much more common would be
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

> @@ -106,8 +98,8 @@ static struct linux_binfmt flat_format = {
>  
>  static int flat_core_dump(struct coredump_params *cprm)
>  {
> -	printk("Process %s:%d received signr %d and should have core dumped\n",
> -			current->comm, current->pid, cprm->siginfo->si_signo);
> +	pr_warning("Process %s:%d received signr %d and should have core dumped\n",
> +		   current->comm, current->pid, cprm->siginfo->si_signo);

Prefer pr_warn

>  	return(1);
>  }
>  
> @@ -190,17 +182,17 @@ static int decompress_exec(
>  	loff_t fpos;
>  	int ret, retval;
>  
> -	DBG_FLT("decompress_exec(offset=%lx,buf=%p,len=%lx)\n",offset, dst, len);
> +	pr_debug("decompress_exec(offset=%lx,buf=%p,len=%lx)\n",offset, dst, len);

Generally unnecessary as the function tracer works well

>  
>  	memset(&strm, 0, sizeof(strm));
>  	strm.workspace = kmalloc(zlib_inflate_workspacesize(), GFP_KERNEL);
>  	if (strm.workspace == NULL) {
> -		DBG_FLT("binfmt_flat: no memory for decompress workspace\n");
> +		pr_debug("no memory for decompress workspace\n");
>  		return -ENOMEM;
>  	}
>  	buf = kmalloc(LBUFSIZE, GFP_KERNEL);
>  	if (buf == NULL) {
> -		DBG_FLT("binfmt_flat: no memory for read buffer\n");
> +		pr_debug("no memory for read buffer\n");

Unnecessary OOM messages as allocs do a stack dump