From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S933310AbdA0Pns (ORCPT <rfc822;w@1wt.eu>);
        Fri, 27 Jan 2017 10:43:48 -0500
Received: from bes.se.axis.com ([195.60.68.10]:36409 "EHLO bes.se.axis.com"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S933050AbdA0Pml (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
        Fri, 27 Jan 2017 10:42:41 -0500
Date: Fri, 27 Jan 2017 16:42:30 +0100
From: Rabin Vincent <rabin.vincent@axis.com>
To: Borislav Petkov <bp@suse.de>
Cc: akpm@linux-foundation.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] printk: fix printk.devkmsg sysctl
Message-ID: <20170127154230.GB3799@axis.com>
References: <1485522706-18852-1-git-send-email-rabin.vincent@axis.com>
 <20170127150141.5w33ardlqab6rekz@pd.tnic>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20170127150141.5w33ardlqab6rekz@pd.tnic>
User-Agent: Mutt/1.5.23 (2014-03-12)
X-TM-AS-GCONF: 00
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Fri, Jan 27, 2017 at 04:01:41PM +0100, Borislav Petkov wrote:
> On Fri, Jan 27, 2017 at 02:11:46PM +0100, Rabin Vincent wrote:
> > @@ -177,7 +177,7 @@ int devkmsg_sysctl_set_loglvl(struct ctl_table *table, int write,
> >  		 * Do not accept an unknown string OR a known string with
> >  		 * trailing crap...
> >  		 */
> > -		if (err < 0 || (err + 1 != *lenp)) {
> 
> Grr, that's that damn '\n'
> 
> echo off > /proc/sys/kernel/printk_devkmsg
> 
> works, of course.
> 
> Ok, I don't want to relax the strncmp() above and would still like to
> return the exact length compared.
> 
> So please change the check above to allow the following inputs:
> 
> 	<str>
> 
> or
> 
> 	<str>\n
> 
> I.e., a trailing, *optional*, '\n' is allowed.

proc_dostring() eats the '\n' and stops after that so we never see it or
what comes after, so we need an strlen():

8<----
>>From ec7e02cdf5b6c9fb1492670928bb7ea4386ca87d Mon Sep 17 00:00:00 2001
From: Rabin Vincent <rabinv@axis.com>
Date: Fri, 27 Jan 2017 14:03:07 +0100
Subject: [PATCHv2] printk: fix printk.devkmsg sysctl

The comment says that it doesn't want to accept trailing crap but the
code does allow it:

 # echo -n offX > /proc/sys/kernel/printk_devkmsg
 #

while at the same time it rejects legitimate uses:

 # echo -n off > /proc/sys/kernel/printk_devkmsg
 -sh: echo: write error: Invalid argument

Fix it.

Before this patch:

 # cat /proc/sys/kernel/printk_devkmsg
 ratelimit
 # echo off > /proc/sys/kernel/printk_devkmsg
 # sysctl -w kernel.printk_devkmsg=off
 sysctl: short write
 # echo -n off > /proc/sys/kernel/printk_devkmsg
 -sh: echo: write error: Invalid argument
 # echo -n offX > /proc/sys/kernel/printk_devkmsg
 #
 # printf "off\nX" >/proc/sys/kernel/printk_devkmsg
 -sh: printf: write error: Invalid argument

After this patch:

 # cat /proc/sys/kernel/printk_devkmsg
 ratelimit
 # echo off > /proc/sys/kernel/printk_devkmsg
 # sysctl -w kernel.printk_devkmsg=off
 kernel.printk_devkmsg = off
 # echo -n off > /proc/sys/kernel/printk_devkmsg
 # echo -n offX > /proc/sys/kernel/printk_devkmsg
 -sh: echo: write error: Invalid argument
 # printf "off\nX" >/proc/sys/kernel/printk_devkmsg
 -sh: printf: write error: Invalid argument

Fixes: 750afe7babd117d ("printk: add kernel parameter to control writes to /dev/kmsg")
Signed-off-by: Rabin Vincent <rabinv@axis.com>
---
 kernel/printk/printk.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 8b26964..935ed71 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -177,7 +177,8 @@ int devkmsg_sysctl_set_loglvl(struct ctl_table *table, int write,
 		 * Do not accept an unknown string OR a known string with
 		 * trailing crap...
 		 */
-		if (err < 0 || (err + 1 != *lenp)) {
+		if (err < 0 || (err != *lenp && err + 1 != *lenp) ||
+		    err != strlen(devkmsg_log_str)) {
 
 			/* ... and restore old setting. */
 			devkmsg_log = old;
-- 
2.1.4