linux-kernel - Re: [PATCH v9 2/3] mmc: core: add random fault injection

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 13 Sep 2011 18:15:03 +0200
From:	Per Forlin <per.forlin@...aro.org>
To:	Akinobu Mita <akinobu.mita@...il.com>
Cc:	Per Forlin <per.forlin@...ricsson.com>, linux-mmc@...r.kernel.org,
	linux-kernel@...r.kernel.org, linaro-dev@...ts.linaro.org,
	Linus Walleij <linus.walleij@...aro.org>,
	Chris Ball <cjb@...top.org>, linux-doc@...r.kernel.org
Subject: Re: [PATCH v9 2/3] mmc: core: add random fault injection

Hi Akinobu,

On 13 September 2011 16:19, Per Forlin <per.forlin@...aro.org> wrote:
> On 13 September 2011 15:12, Akinobu Mita <akinobu.mita@...il.com> wrote:
>> 2011/8/19 Per Forlin <per.forlin@...ricsson.com>:
>>
>>> +#ifdef KERNEL
>>> +/*
>>> + * Internal function. Pass the boot param fail_mmc_request to
>>> + * the setup fault injection attributes routine.
>>> + */
>>> +static int __init setup_fail_mmc_request(char *str)
>>> +{
>>> +       return setup_fault_attr(&fail_mmc_request, str);
>>> +}
>>> +__setup("fail_mmc_request=", setup_fail_mmc_request);
>>> +#endif /* KERNEL */
>>
>> You attempt to enable __setup() only when mmc_core is built into
>> the kernel.  Does it really work? I cannot find any drivers using
>> "KERNEL" macro.
>>
> Your right it doesn't work. I think I change from ifndef MODULE to
> ifdef KERNEL at one point.
> It should be "ifndef MODULE"
>
>> You can use module_param_cb() instead of __setup() without #ifdef KERNEL.
>> When mmc_core is built into the kernel, you can specify the parameter
>> with "mmc_core.fail_mmc_request=..."
>>
I am considering using module_param() with perm = 0 (not visible in
sysfs). The purpose of the param is to set fault attributes during
kerne boot time or module load time. After the kernel boot all can be
set under debug fs, therefore no need to make the module param
visible.

What do you think about this? I have not tested it yet.
----------------------------------------------------------
+++ b/drivers/mmc/core/debugfs.c
@@ -20,6 +20,14 @@
 #include "core.h"
 #include "mmc_ops.h"

+#ifdef CONFIG_FAIL_MMC_REQUEST
+
+static DECLARE_FAULT_ATTR(fail_mmc_default_attr);
+static char *fail_mmc_request;
+module_param(fail_mmc_request, charp, 0);
+
+#endif /* CONFIG_FAIL_MMC_REQUEST */
+
 /* The debugfs functions are optimized away when CONFIG_DEBUG_FS isn't set. */
 static int mmc_ios_show(struct seq_file *s, void *data)
 {
@@ -159,23 +167,6 @@ static int mmc_clock_opt_set(void *data, u64 val)
        return 0;
 }

-#ifdef CONFIG_FAIL_MMC_REQUEST
-
-static DECLARE_FAULT_ATTR(fail_mmc_request);
-
-#ifdef KERNEL
-/*
- * Internal function. Pass the boot param fail_mmc_request to
- * the setup fault injection attributes routine.
- */
-static int __init setup_fail_mmc_request(char *str)
-{
-       return setup_fault_attr(&fail_mmc_request, str);
-}
-__setup("fail_mmc_request=", setup_fail_mmc_request);
-#endif /* KERNEL */
-#endif /* CONFIG_FAIL_MMC_REQUEST */
-
 DEFINE_SIMPLE_ATTRIBUTE(mmc_clock_fops, mmc_clock_opt_get, mmc_clock_opt_set,
        "%llu\n");

@@ -207,7 +198,9 @@ void mmc_add_host_debugfs(struct mmc_host *host)
                goto err_node;
 #endif
 #ifdef CONFIG_FAIL_MMC_REQUEST
-       host->fail_mmc_request = fail_mmc_request;
+       if (fail_mmc_request)
+               setup_fault_attr(&fail_mmc_default_attr, fail_mmc_request);
+       host->fail_mmc_request = fail_mmc_default_attr;
        if (IS_ERR(fault_create_debugfs_attr("fail_mmc_request",
                                             root,
                                             &host->fail_mmc_request)))
----------------------------------------------------------
It's only necessary to call setup_fault_attr() once for all hosts.
Here it's called one time for each host. I think it's ok since the
routine is small and used for debugging purposes.
I could use a static bool to indicate whether setup_fault_attr() has
already been issued.
+ if (fail_mmc_request && !setup_fault_attr_done)

Regards,
Per
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/