From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.2 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 99D75C43331 for ; Thu, 26 Mar 2020 09:30:09 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 3B3AC2073E for ; Thu, 26 Mar 2020 09:30:09 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 3B3AC2073E Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=suse.cz Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 8BB6C6B000A; Thu, 26 Mar 2020 05:30:08 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 86AE66B000C; Thu, 26 Mar 2020 05:30:08 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 759E06B000D; Thu, 26 Mar 2020 05:30:08 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0084.hostedemail.com [216.40.44.84]) by kanga.kvack.org (Postfix) with ESMTP id 5F1236B000A for ; Thu, 26 Mar 2020 05:30:08 -0400 (EDT) Received: from smtpin23.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id 32EFE824934B for ; Thu, 26 Mar 2020 09:30:08 +0000 (UTC) X-FDA: 76636992096.23.art90_12704b8d3934 X-HE-Tag: art90_12704b8d3934 X-Filterd-Recvd-Size: 3861 Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by imf43.hostedemail.com (Postfix) with ESMTP for ; Thu, 26 Mar 2020 09:30:07 +0000 (UTC) X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 3DF79AD08; Thu, 26 Mar 2020 09:30:04 +0000 (UTC) Subject: Re: [RFC v2 1/2] kernel/sysctl: support setting sysctl parameters from kernel command line To: Kees Cook Cc: Luis Chamberlain , Iurii Zaikin , linux-kernel@vger.kernel.org, linux-api@vger.kernel.org, linux-mm@kvack.org, Ivan Teterevkov , Michal Hocko , David Rientjes , Matthew Wilcox , "Eric W . Biederman" , "Guilherme G . Piccoli" References: <20200325120345.12946-1-vbabka@suse.cz> <202003251404.12A69348@keescook> From: Vlastimil Babka Message-ID: Date: Thu, 26 Mar 2020 10:30:02 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 MIME-Version: 1.0 In-Reply-To: <202003251404.12A69348@keescook> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On 3/25/20 10:21 PM, Kees Cook wrote: >> --- a/init/main.c >> +++ b/init/main.c >> @@ -1345,6 +1345,25 @@ void __weak free_initmem(void) >> free_initmem_default(POISON_FREE_INITMEM); >> } >> >> +static void do_sysctl_args(void) >> +{ >> +#ifdef CONFIG_SYSCTL >> + size_t len = strlen(saved_command_line) + 1; >> + char *command_line; >> + >> + command_line = kzalloc(len, GFP_KERNEL); >> + if (!command_line) >> + panic("%s: Failed to allocate %zu bytes\n", __func__, len); >> + >> + strcpy(command_line, saved_command_line); > > No need to open-code this: > > char *command_line; > > command_line = kstrdup(saved_command_line, GFP_KERNEL); > if (!command_line) > panic("%s: Failed to allocate %zu bytes\n", __func__, len); > Ah, right. I admit I basically copy_pasted some other parse_args user. >> diff --git a/kernel/sysctl.c b/kernel/sysctl.c >> index ad5b88a53c5a..18c7f5606d55 100644 >> --- a/kernel/sysctl.c >> +++ b/kernel/sysctl.c >> @@ -1980,6 +1980,68 @@ int __init sysctl_init(void) >> return 0; >> } >> >> +/* Set sysctl value passed on kernel command line. */ >> +int process_sysctl_arg(char *param, char *val, >> + const char *unused, void *arg) >> +{ >> + size_t count; >> + char *remaining; >> + int err; >> + loff_t ppos = 0; >> + struct ctl_table *ctl, *found = NULL; >> + >> + if (strncmp(param, "sysctl.", sizeof("sysctl.") - 1)) >> + return 0; >> + >> + param += sizeof("sysctl.") - 1; >> + >> + remaining = param; >> + ctl = &sysctl_base_table[0]; >> + >> + while(ctl->procname != 0) { >> + int len = strlen(ctl->procname); >> + if (strncmp(remaining, ctl->procname, len)) { >> + ctl++; >> + continue; >> + } > > I think you need to validate that "len" is within "remaining" here > first. My reasoning was that if remaining terminates too early, the null byte would be different from non-null byte in ctl->procname and thus strncmp will return it as different? And the reason I used len in strncmp there is only so it doesn't compare the terminating null, because remaning can continue with ".foo" instead. >> + if (ctl->child) { >> + if (remaining[len] == '.') { >> + remaining += len + 1; > > And that "len + 1" is still valid. And since we passed strncmp(..., len), remaining[len] might be null byte, but then we can still compare it with '.'. But C strings are full of landmines.