Received: by 2002:a05:6a10:6d10:0:0:0:0 with SMTP id gq16csp939915pxb; Fri, 22 Apr 2022 14:54:05 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwmtGycWYtGtMOJC9TL886u+nXDXaDz4T7temQPZuA2N5OWR/cW6L2vxdoNOw6JR9vHBMbx X-Received: by 2002:a05:6a00:1a0a:b0:4fc:d6c5:f3f1 with SMTP id g10-20020a056a001a0a00b004fcd6c5f3f1mr7168022pfv.45.1650664445169; Fri, 22 Apr 2022 14:54:05 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1650664445; cv=none; d=google.com; s=arc-20160816; b=qtzB9jL+0LTzUrV0JB+2Tf7MzauJzgNmomIrFf1RIKb5ddE1jiaR56isD1ha4BGiqC 3RdSSt2RGJEMNf3p02VFNL2tWbB/l2n98jmca2RmIBqkKzUXcF3RdnQQybut6ou3kTd8 FPGv63nIK+pUlx8YMHexeCZRfeJPs7EmSQSHo+8fcPK2bdg4sk8lxughJRQutokN0fqr e+BWr18Te+ROQObxVUCZG/A+8n4Te2QR3vSr/FbyQ8i11zKUdAXlUj9T9/CuhSjNh8o8 muWZY1ElFI60UkS8h8ncUaOzSixfDLd1oDvpFFWAsXpnIWyPHLxpC3ju2OWnmnVejCkx z0Uw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=y1izGaPHTx8ZX9q7VfNwdJR0QUIA8B8JKj1kURkLhVk=; b=qnlmbZ9jzpAAHX8yZsulY1f3SnpAjpG7d17dLv5HKZwdYuvp7sZ+UjbmX6RGMkHPqz 9CHZCtk5Lm2WR43fBuI2EFqDrXK4SrWbLfW+m59R5GggMr+w6bhr30qERDDX9TQ6TvrW 9FvtDVx5eY90gutap6943z6xveH4ltDLWkAAkaXZ6DfbsxoFGSEoUmA9sdIoXY4Px2iA 6d5bN/gWtBWSeC5dQFA3YaB6jIM4nDOfenCzA2FDbMC6ICwbxTSsa11GgRkB3zCT12ZY 49TnzXEN3TrpTwISmVQGJ7eSPI6khPB9UTQZC3ha+0FiLdTI9h4FyGpJhsTIjYUuMIHO DOvQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@natalenko.name header.s=dkim-20170712 header.b=BdJ5dahq; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=natalenko.name Return-Path: Received: from lindbergh.monkeyblade.net (lindbergh.monkeyblade.net. [2620:137:e000::1:18]) by mx.google.com with ESMTPS id v4-20020a17090a6b0400b001cba0ee7abfsi8912791pjj.113.2022.04.22.14.54.04 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 22 Apr 2022 14:54:05 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18; Authentication-Results: mx.google.com; dkim=pass header.i=@natalenko.name header.s=dkim-20170712 header.b=BdJ5dahq; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=natalenko.name Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id 357D0354195; Fri, 22 Apr 2022 13:01:44 -0700 (PDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1389423AbiDUOcS (ORCPT + 99 others); Thu, 21 Apr 2022 10:32:18 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:35942 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1389379AbiDUOcJ (ORCPT ); Thu, 21 Apr 2022 10:32:09 -0400 Received: from vulcan.natalenko.name (vulcan.natalenko.name [104.207.131.136]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 8452D44744 for ; Thu, 21 Apr 2022 07:28:38 -0700 (PDT) Received: from spock.localnet (unknown [83.148.33.151]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by vulcan.natalenko.name (Postfix) with ESMTPSA id 6ECEEE9E396; Thu, 21 Apr 2022 16:28:34 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=natalenko.name; s=dkim-20170712; t=1650551314; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=y1izGaPHTx8ZX9q7VfNwdJR0QUIA8B8JKj1kURkLhVk=; b=BdJ5dahqbUG8L1PJ/+DtAEPNiEQtdieHtj/VXTX2vA/2Ct1lacqutHjSzo0mtPNboktvEU fPlh2h93HUv3D4Zn9ilGfJ0UkBknZfgAYM/Xw+YDFobbwKqv7nqEPNquwUHcpmP/dYcD9G cCCj25ZSfGfnQbUIGKNscNBiOYKlQpM= From: Oleksandr Natalenko To: mcgrof@kernel.org, Aaron Tomlin Cc: cl@linux.com, pmladek@suse.com, mbenes@suse.cz, christophe.leroy@csgroup.eu, akpm@linux-foundation.org, linux-kernel@vger.kernel.org, linux-modules@vger.kernel.org, atomlin@atomlin.com, ghalat@redhat.com, neelx@redhat.com Subject: Re: [PATCH v3 2/2] module: Introduce module unload taint tracking Date: Thu, 21 Apr 2022 16:28:32 +0200 Message-ID: <2100545.irdbgypaU6@natalenko.name> In-Reply-To: <20220420115257.3498300-3-atomlin@redhat.com> References: <20220420115257.3498300-1-atomlin@redhat.com> <20220420115257.3498300-3-atomlin@redhat.com> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,RDNS_NONE,SPF_HELO_NONE autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hello. Thanks for this submission. Please see one comment inline. On st=C5=99eda 20. dubna 2022 13:52:57 CEST Aaron Tomlin wrote: > Currently, only the initial module that tainted the kernel is > recorded e.g. when an out-of-tree module is loaded. >=20 > The purpose of this patch is to allow the kernel to maintain a record of > each unloaded module that taints the kernel. So, in addition to > displaying a list of linked modules (see print_modules()) e.g. in the > event of a detected bad page, unloaded modules that carried a taint/or > taints are displayed too. A tainted module unload count is maintained. >=20 > The number of tracked modules is not fixed. This feature is disabled by > default. >=20 > Signed-off-by: Aaron Tomlin > --- > init/Kconfig | 11 ++++++++ > kernel/module/main.c | 65 ++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 76 insertions(+) >=20 > diff --git a/init/Kconfig b/init/Kconfig > index ddcbefe535e9..6b30210f787d 100644 > --- a/init/Kconfig > +++ b/init/Kconfig > @@ -2118,6 +2118,17 @@ config MODULE_FORCE_UNLOAD > rmmod). This is mainly for kernel developers and desperate users. > If unsure, say N. > =20 > +config MODULE_UNLOAD_TAINT_TRACKING > + bool "Tainted module unload tracking" > + depends on MODULE_UNLOAD > + default n > + help > + This option allows you to maintain a record of each unloaded > + module that tainted the kernel. In addition to displaying a > + list of linked (or loaded) modules e.g. on detection of a bad > + page (see bad_page()), the aforementioned details are also > + shown. If unsure, say N. > + > config MODVERSIONS > bool "Module versioning support" > help > diff --git a/kernel/module/main.c b/kernel/module/main.c > index ea78cec316dd..d7cc64172dd1 100644 > --- a/kernel/module/main.c > +++ b/kernel/module/main.c > @@ -68,6 +68,16 @@ > */ > DEFINE_MUTEX(module_mutex); > LIST_HEAD(modules); > +#ifdef CONFIG_MODULE_UNLOAD_TAINT_TRACKING > +static LIST_HEAD(unloaded_tainted_modules); > + > +struct mod_unload_taint { > + struct list_head list; > + char name[MODULE_NAME_LEN]; > + unsigned long taints; > + u64 count; > +}; > +#endif > =20 > /* Work queue for freeing init sections in success case */ > static void do_free_init(struct work_struct *w); > @@ -150,6 +160,41 @@ int unregister_module_notifier(struct notifier_block= *nb) > } > EXPORT_SYMBOL(unregister_module_notifier); > =20 > +#ifdef CONFIG_MODULE_UNLOAD_TAINT_TRACKING > +static int try_add_tainted_module(struct module *mod) > +{ > + struct mod_unload_taint *mod_taint; > + > + module_assert_mutex_or_preempt(); > + > + list_for_each_entry_rcu(mod_taint, &unloaded_tainted_modules, list, > + lockdep_is_held(&module_mutex)) { > + size_t len =3D strlen(mod_taint->name); > + > + if (len =3D=3D strlen(mod->name) && !memcmp(mod_taint->name, mod->name= , len) && Here, two strings are compared, so I'd expect to see `strncmp()` instead of= `memcmp()`. > + mod_taint->taints & mod->taints) { > + mod_taint->count++; > + goto out; > + } > + } > + > + mod_taint =3D kmalloc(sizeof(*mod_taint), GFP_KERNEL); > + if (unlikely(!mod_taint)) > + return -ENOMEM; > + strscpy(mod_taint->name, mod->name, MODULE_NAME_LEN); > + mod_taint->taints =3D mod->taints; > + list_add_rcu(&mod_taint->list, &unloaded_tainted_modules); > + mod_taint->count =3D 1; > +out: > + return 0; > +} > +#else /* MODULE_UNLOAD_TAINT_TRACKING */ > +static int try_add_tainted_module(struct module *mod) > +{ > + return 0; > +} > +#endif > + > /* > * We require a truly strong try_module_get(): 0 means success. > * Otherwise an error is returned due to ongoing or failed > @@ -1201,6 +1246,9 @@ static void free_module(struct module *mod) > module_bug_cleanup(mod); > /* Wait for RCU-sched synchronizing before releasing mod->list and bugl= ist. */ > synchronize_rcu(); > + if (try_add_tainted_module(mod)) > + pr_err("%s: adding tainted module to the unloaded tainted modules list= failed.\n", > + mod->name); > mutex_unlock(&module_mutex); > =20 > /* Clean up CFI for the module. */ > @@ -3126,6 +3174,9 @@ struct module *__module_text_address(unsigned long = addr) > void print_modules(void) > { > struct module *mod; > +#ifdef CONFIG_MODULE_UNLOAD_TAINT_TRACKING > + struct mod_unload_taint *mod_taint; > +#endif > char buf[MODULE_FLAGS_BUF_SIZE]; > =20 > printk(KERN_DEFAULT "Modules linked in:"); > @@ -3136,6 +3187,20 @@ void print_modules(void) > continue; > pr_cont(" %s%s", mod->name, module_flags(mod, buf)); > } > +#ifdef CONFIG_MODULE_UNLOAD_TAINT_TRACKING > + if (!list_empty(&unloaded_tainted_modules)) { > + printk(KERN_DEFAULT "Unloaded tainted modules:"); > + list_for_each_entry_rcu(mod_taint, &unloaded_tainted_modules, > + list) { > + size_t l; > + > + l =3D module_flags_taint(mod_taint->taints, buf); > + buf[l++] =3D '\0'; > + pr_cont(" %s(%s):%llu", mod_taint->name, buf, > + mod_taint->count); > + } > +#endif > + } > preempt_enable(); > if (last_unloaded_module[0]) > pr_cont(" [last unloaded: %s]", last_unloaded_module); >=20 =2D-=20 Oleksandr Natalenko (post-factum)