From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752237AbdI0IKg (ORCPT ); Wed, 27 Sep 2017 04:10:36 -0400 Received: from mx2.suse.de ([195.135.220.15]:36443 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752173AbdI0IKd (ORCPT ); Wed, 27 Sep 2017 04:10:33 -0400 Date: Wed, 27 Sep 2017 10:10:31 +0200 From: Jean Delvare To: LKML Cc: Andrew Morton , Ingo Molnar , Baoquan He Subject: [PATCH] params: Fix an overflow in param_attr_show Message-ID: <20170927101031.7a3b2398@endymion> Organization: SUSE Linux X-Mailer: Claws Mail 3.13.2 (GTK+ 2.24.31; x86_64-suse-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Function param_attr_show could overflow the buffer it is operating on. The buffer size is PAGE_SIZE, and the string returned by attribute->param->ops->get is generated by scnprintf(buffer, PAGE_SIZE, ...) so it could be PAGE_SIZE - 1 long, with the terminating '\0' at the very end of the buffer. Calling strcat(..., "\n") on this isn't safe, as the '\0' will be replaced by '\n' (OK) and then another '\0' will be added past the end of the buffer (not OK.) Simply add the trailing '\n' when writing the attribute contents to the buffer originally. This is safe, and also faster. Credits to Teradata for discovering this issue. Signed-off-by: Jean Delvare --- kernel/params.c | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) --- linux-4.13.orig/kernel/params.c 2017-09-19 16:07:18.794254776 +0200 +++ linux-4.13/kernel/params.c 2017-09-19 16:12:57.398426205 +0200 @@ -236,14 +236,14 @@ char *parse_args(const char *doing, EXPORT_SYMBOL(param_ops_##name) -STANDARD_PARAM_DEF(byte, unsigned char, "%hhu", kstrtou8); -STANDARD_PARAM_DEF(short, short, "%hi", kstrtos16); -STANDARD_PARAM_DEF(ushort, unsigned short, "%hu", kstrtou16); -STANDARD_PARAM_DEF(int, int, "%i", kstrtoint); -STANDARD_PARAM_DEF(uint, unsigned int, "%u", kstrtouint); -STANDARD_PARAM_DEF(long, long, "%li", kstrtol); -STANDARD_PARAM_DEF(ulong, unsigned long, "%lu", kstrtoul); -STANDARD_PARAM_DEF(ullong, unsigned long long, "%llu", kstrtoull); +STANDARD_PARAM_DEF(byte, unsigned char, "%hhu\n", kstrtou8); +STANDARD_PARAM_DEF(short, short, "%hi\n", kstrtos16); +STANDARD_PARAM_DEF(ushort, unsigned short, "%hu\n", kstrtou16); +STANDARD_PARAM_DEF(int, int, "%i\n", kstrtoint); +STANDARD_PARAM_DEF(uint, unsigned int, "%u\n", kstrtouint); +STANDARD_PARAM_DEF(long, long, "%li\n", kstrtol); +STANDARD_PARAM_DEF(ulong, unsigned long, "%lu\n", kstrtoul); +STANDARD_PARAM_DEF(ullong, unsigned long long, "%llu\n", kstrtoull); int param_set_charp(const char *val, const struct kernel_param *kp) { @@ -270,7 +270,7 @@ EXPORT_SYMBOL(param_set_charp); int param_get_charp(char *buffer, const struct kernel_param *kp) { - return scnprintf(buffer, PAGE_SIZE, "%s", *((char **)kp->arg)); + return scnprintf(buffer, PAGE_SIZE, "%s\n", *((char **)kp->arg)); } EXPORT_SYMBOL(param_get_charp); @@ -549,10 +549,6 @@ static ssize_t param_attr_show(struct mo kernel_param_lock(mk->mod); count = attribute->param->ops->get(buf, attribute->param); kernel_param_unlock(mk->mod); - if (count > 0) { - strcat(buf, "\n"); - ++count; - } return count; } -- Jean Delvare SUSE L3 Support