linux-kernel - Re: [PATCH] printk: remove unnecessary kmalloc() from syslog during clear

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: <20180625133705.7zsg2rkw4xtyg6ie@pathway.suse.cz>
Date:   Mon, 25 Jun 2018 15:37:05 +0200
From:   Petr Mladek <pmladek@...e.com>
To:     Namit Gupta <gupta.namit@...sung.com>
Cc:     sergey.senozhatsky@...il.com, rostedt@...dmis.org,
        linux-kernel@...r.kernel.org, pankaj.m@...sung.com,
        a.sahrawat@...sung.com, himanshu.m@...sung.com
Subject: Re: [PATCH] printk: remove unnecessary kmalloc() from syslog during
 clear

On Wed 2018-06-20 19:26:19, Namit Gupta wrote:
> When the request is only for clearing logs, there is no need for
> allocation/deallocation. Only the indexes need to be reset and returned.
> Rest of the patch is mostly made up of changes because of indention.
> 
> Signed-off-by: Namit Gupta <gupta.namit@...sung.com>
> Signed-off-by: Himanshu Maithani <himanshu.m@...sung.com>

> ---
>  kernel/printk/printk.c | 111 ++++++++++++++++++++++++++-----------------------
>  1 file changed, 60 insertions(+), 51 deletions(-)
> 
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 512f7c2..53952ce 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -1348,71 +1348,80 @@ static int syslog_print_all(char __user *buf, int size, bool clear)
>  {
>  	char *text;
>  	int len = 0;
> +	u64 next_seq;
> +	u64 seq;
> +	u32 idx;
> +
> +	if (!buf) {
> +		if (clear) {
> +			logbuf_lock_irq();
> +			clear_seq = log_next_seq;
> +			clear_idx = log_next_idx;
> +			logbuf_unlock_irq();

I pushed a bit different version into printk.git, branch for-4.19,
see below. It removes the code duplication. Also it keeps the original
indentation. IMHO, it helped to better distinguish the code for printing
and clearing.

It is rather a cosmetic change, so I do not want you to resend
Reviewed-by tags. But feel free to disagree and ask me to use
the original variant.


This is in printk.git now:

>From 41cb6dcedd9257d51fd310bf9b2958d11d93aa2b Mon Sep 17 00:00:00 2001
From: Namit Gupta <gupta.namit@...sung.com>
Date: Mon, 25 Jun 2018 14:58:05 +0200
Subject: [PATCH] printk: remove unnecessary kmalloc() from syslog during

When the request is only for clearing logs, there is no need for
allocation/deallocation. Only the indexes need to be reset and returned.
Rest of the patch is mostly made up of changes because of indention.

Link: http://lkml.kernel.org/r/20180620135951epcas5p3bd2a8f25ec689ca333bce861b527dba2~54wyKcT0_3155531555epcas5p3y@epcas5p3.samsung.com
Cc: linux-kernel@...r.kernel.org
Cc: pankaj.m@...sung.com
Cc: a.sahrawat@...sung.com
Signed-off-by: Namit Gupta <gupta.namit@...sung.com>
Signed-off-by: Himanshu Maithani <himanshu.m@...sung.com>
Reviewed-by: Steven Rostedt (VMware) <rostedt@...dmis.org>
Reviewed-by: Sergey Senozhatsky <sergey.senozhatsky@...il.com>
[pmladek@...e.com: Removed code duplication.]
Signed-off-by: Petr Mladek <pmladek@...e.com>
---
 kernel/printk/printk.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 247808333ba4..0fa2ca6fd8f9 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -1350,12 +1350,14 @@ static int syslog_print(char __user *buf, int size)
 
 static int syslog_print_all(char __user *buf, int size, bool clear)
 {
-	char *text;
+	char *text = NULL;
 	int len = 0;
 
-	text = kmalloc(LOG_LINE_MAX + PREFIX_MAX, GFP_KERNEL);
-	if (!text)
-		return -ENOMEM;
+	if (buf) {
+		text = kmalloc(LOG_LINE_MAX + PREFIX_MAX, GFP_KERNEL);
+		if (!text)
+			return -ENOMEM;
+	}
 
 	logbuf_lock_irq();
 	if (buf) {
@@ -1426,7 +1428,8 @@ static int syslog_print_all(char __user *buf, int size, bool clear)
 	}
 	logbuf_unlock_irq();
 
-	kfree(text);
+	if (text)
+		kfree(text);
 	return len;
 }
 
-- 
2.13.7