From patchwork Wed Aug 20 16:12:26 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Rusty Russell X-Patchwork-Id: 4753491 Return-Path: X-Original-To: patchwork-intel-gfx@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 3C0BC9F344 for ; Wed, 20 Aug 2014 19:47:10 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 0170C2015E for ; Wed, 20 Aug 2014 19:47:09 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id DFC0520155 for ; Wed, 20 Aug 2014 19:47:07 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 65D1B6E76B; Wed, 20 Aug 2014 12:47:07 -0700 (PDT) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from ozlabs.org (ozlabs.org [103.22.144.67]) by gabe.freedesktop.org (Postfix) with ESMTP id 282C56E76B for ; Wed, 20 Aug 2014 12:47:05 -0700 (PDT) Received: by ozlabs.org (Postfix, from userid 1011) id 7B00414012F; Thu, 21 Aug 2014 05:47:02 +1000 (EST) From: Rusty Russell To: Daniel Vetter In-Reply-To: References: <87wqac87dq.fsf@rustcorp.com.au> User-Agent: Notmuch/0.17 (http://notmuchmail.org) Emacs/24.3.1 (x86_64-pc-linux-gnu) Date: Thu, 21 Aug 2014 01:42:26 +0930 Message-ID: <87ppfvi1hx.fsf@rustcorp.com.au> MIME-Version: 1.0 Cc: Li Zhong , Jani Nikula , intel-gfx , Linux Kernel Mailing List , Jean Delvare , Andrew Morton , Jon Mason Subject: Re: [Intel-gfx] [PATCH 0/4] module: add support for unsafe, tainting parameters X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Spam-Status: No, score=-3.3 required=5.0 tests=BAYES_00, DATE_IN_PAST_03_06, RCVD_IN_DNSWL_MED,RP_MATCHES_RCVD,UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Daniel Vetter writes: > On Wed, Aug 13, 2014 at 10:25 PM, Rusty Russell wrote: >> Jani Nikula writes: >>> This is a generic version of Daniel's patch [1] letting us have unsafe >>> module parameters (experimental, debugging, testing, etc.) that taint >>> the kernel when set. Quoting Daniel, >> >> OK, I think the idea is fine, but we'll probably only want this for >> a few types (eg. int and bool). So for the moment I prefer a more >> naive approach. >> >> Does this work for you? > > Can you please discuss this with yourself from a few months back? > We've done the general version since you suggested that just doing it > for int is a bit lame ;-) And I actually agreed so asked Jani to look > into that. Don't listen to me, I'm an idiot! Applied. I've applied this cleanup on top, however. Cheers, Rusty. Subject: param: check for tainting before calling set op. This means every set op doesn't need to call it, and it can move into params.c. Signed-off-by: Rusty Russell diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h index 9531f9f9729e..593501996574 100644 --- a/include/linux/moduleparam.h +++ b/include/linux/moduleparam.h @@ -374,22 +374,6 @@ static inline void destroy_params(const struct kernel_param *params, #define __param_check(name, p, type) \ static inline type __always_unused *__check_##name(void) { return(p); } -/** - * param_check_unsafe - Warn and taint the kernel if setting dangerous options. - * - * This gets called from all the standard param setters, but can be used from - * custom setters as well. - */ -static inline void -param_check_unsafe(const struct kernel_param *kp) -{ - if (kp->flags & KERNEL_PARAM_FL_UNSAFE) { - pr_warn("Setting dangerous option %s - tainting kernel\n", - kp->name); - add_taint(TAINT_USER, LOCKDEP_STILL_OK); - } -} - extern struct kernel_param_ops param_ops_byte; extern int param_set_byte(const char *val, const struct kernel_param *kp); extern int param_get_byte(char *buffer, const struct kernel_param *kp); diff --git a/kernel/params.c b/kernel/params.c index ad8d04563c3a..f3cc977d6a66 100644 --- a/kernel/params.c +++ b/kernel/params.c @@ -83,6 +83,15 @@ bool parameq(const char *a, const char *b) return parameqn(a, b, strlen(a)+1); } +static void param_check_unsafe(const struct kernel_param *kp) +{ + if (kp->flags & KERNEL_PARAM_FL_UNSAFE) { + pr_warn("Setting dangerous option %s - tainting kernel\n", + kp->name); + add_taint(TAINT_USER, LOCKDEP_STILL_OK); + } +} + static int parse_one(char *param, char *val, const char *doing, @@ -109,6 +119,7 @@ static int parse_one(char *param, pr_debug("handling %s with %p\n", param, params[i].ops->set); mutex_lock(¶m_lock); + param_check_unsafe(¶ms[i]); err = params[i].ops->set(val, ¶ms[i]); mutex_unlock(¶m_lock); return err; @@ -233,7 +244,6 @@ char *parse_args(const char *doing, #define STANDARD_PARAM_DEF(name, type, format, strtolfn) \ int param_set_##name(const char *val, const struct kernel_param *kp) \ { \ - param_check_unsafe(kp); \ return strtolfn(val, 0, (type *)kp->arg); \ } \ int param_get_##name(char *buffer, const struct kernel_param *kp) \ @@ -266,8 +276,6 @@ int param_set_charp(const char *val, const struct kernel_param *kp) return -ENOSPC; } - param_check_unsafe(kp); - maybe_kfree_parameter(*(char **)kp->arg); /* This is a hack. We can't kmalloc in early boot, and we @@ -305,8 +313,6 @@ EXPORT_SYMBOL(param_ops_charp); /* Actually could be a bool or an int, for historical reasons. */ int param_set_bool(const char *val, const struct kernel_param *kp) { - param_check_unsafe(kp); - /* No equals means "set"... */ if (!val) val = "1"; @@ -336,8 +342,6 @@ int param_set_invbool(const char *val, const struct kernel_param *kp) bool boolval; struct kernel_param dummy; - param_check_unsafe(kp); - dummy.arg = &boolval; ret = param_set_bool(val, &dummy); if (ret == 0) @@ -364,8 +368,6 @@ int param_set_bint(const char *val, const struct kernel_param *kp) bool v; int ret; - param_check_unsafe(kp); - /* Match bool exactly, by re-using it. */ boolkp = *kp; boolkp.arg = &v; @@ -485,8 +487,6 @@ int param_set_copystring(const char *val, const struct kernel_param *kp) { const struct kparam_string *kps = kp->str; - param_check_unsafe(kp); - if (strlen(val)+1 > kps->maxlen) { pr_err("%s: string doesn't fit in %u chars.\n", kp->name, kps->maxlen-1); @@ -563,6 +563,7 @@ static ssize_t param_attr_store(struct module_attribute *mattr, return -EPERM; mutex_lock(¶m_lock); + param_check_unsafe(attribute->param); err = attribute->param->ops->set(buf, attribute->param); mutex_unlock(¶m_lock); if (!err)