| 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
| ||
|
Date: Fri, 29 May 2015 15:49:43 +0200
From: Borislav Petkov <bp@...en8.de>
To: Aravind Gopalakrishnan <Aravind.Gopalakrishnan@....com>
Cc: dougthompson@...ssion.com, mchehab@....samsung.com,
linux-edac@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/6] edac, mce_amd_inj: Modify flags attrigute to use
string arguments
On Wed, May 27, 2015 at 02:03:35PM -0500, Aravind Gopalakrishnan wrote:
> Use char values such as "hw" or "sw" to indicate the type of error
> injection to be performed.
>
> Current flags attribute derives the meanings of values that can be
> programmed into it from asm/mce.h. Moving to defined strings for the
> atribute allows this module to be self sufficient and removes the
> dependency. Also, we can introduce new flags as and when needed without
> having to worry about conflicting with the flags already defined
> in asm/mce.h
>
> Also, modify do_inject() to use the newly defined injection_type enum
> to figure out the injection mechanism we need to use
>
> Signed-off-by: Aravind Gopalakrishnan <Aravind.Gopalakrishnan@....com>
> ---
> drivers/edac/mce_amd_inj.c | 85 +++++++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 76 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/edac/mce_amd_inj.c b/drivers/edac/mce_amd_inj.c
> index 15f6aa1..daec4af 100644
> --- a/drivers/edac/mce_amd_inj.c
> +++ b/drivers/edac/mce_amd_inj.c
> @@ -15,6 +15,8 @@
> #include <linux/device.h>
> #include <linux/module.h>
> #include <linux/cpu.h>
> +#include <linux/string.h>
> +#include <linux/uaccess.h>
> #include <asm/mce.h>
>
> #include "mce_amd.h"
> @@ -27,6 +29,23 @@ static struct dentry *dfs_inj;
>
> static u8 n_banks;
>
> +#define MAX_FLAG_OPT_SIZE 10
Why 10?
This should be 2 and increased when another, longer injection type
string gets introduced.
> +
> +enum injection_type {
> + SW_INJ = 0, /* SW injection, simply decode the error */
> + HW_INJ, /* Trigger a #MC */
> + N_INJ_TYPES,
> +};
> +
> +static const char * const flags_options[] = {
> + [SW_INJ] = "sw",
> + [HW_INJ] = "hw",
> + NULL
> +};
> +
> +/* Set default injection to SW_INJ */
> +enum injection_type inj_type = SW_INJ;
> +
> #define MCE_INJECT_SET(reg) \
> static int inj_##reg##_set(void *data, u64 val) \
> { \
> @@ -81,24 +100,72 @@ static int toggle_hw_mce_inject(unsigned int cpu, bool enable)
> return err;
> }
>
> -static int flags_get(void *data, u64 *val)
> +static int __set_inj(const char *buf, size_t cnt)
> {
> - struct mce *m = (struct mce *)data;
> + int i;
> + const char *inj_op = NULL;
>
> - *val = m->inject_flags;
> + for (i = 0; i <= N_INJ_TYPES; i++) {
> + inj_op = flags_options[i];
> + if (inj_op && strncmp(inj_op, buf, cnt) == 0)
> + break;
> + }
What's wrong with:
for (i = 0; i < N_INJ_TYPES; i++) {
if (!strncmp(flags_options[i], buf, strlen(flags_options[i]))) {
inj_type = i;
return 0;
}
}
return -EINVAL;
> +
> + if (!inj_op)
> + return -EINVAL;
> +
> + inj_type = i;
>
> return 0;
> }
>
> -static int flags_set(void *data, u64 val)
> +static ssize_t flags_read(struct file *filp, char __user *ubuf,
> + size_t cnt, loff_t *ppos)
> {
> - struct mce *m = (struct mce *)data;
> + char buf[MAX_FLAG_OPT_SIZE + 1];
> + int n;
>
> - m->inject_flags = (u8)val;
> - return 0;
> + n = sprintf(buf, "%s\n", flags_options[inj_type]);
> +
> + return simple_read_from_buffer(ubuf, cnt, ppos, buf, n);
> }
>
> -DEFINE_SIMPLE_ATTRIBUTE(flags_fops, flags_get, flags_set, "%llu\n");
> +static ssize_t flags_write(struct file *filp, const char __user *ubuf,
> + size_t cnt, loff_t *ppos)
> +{
> + char buf[MAX_FLAG_OPT_SIZE + 1];
> + int err;
> + size_t ret;
> +
> + ret = cnt;
You're assigning cnt to ret here...
> +
> + if (cnt > MAX_FLAG_OPT_SIZE)
> + cnt = MAX_FLAG_OPT_SIZE;
... but correcting cnt afterwards. The assignment should be *after* that
correction.
> +
> + if (copy_from_user(&buf, ubuf, cnt))
> + return -EFAULT;
> +
> + buf[cnt] = 0;
> +
> + /* strip whitespaces.. */
> + strstrip(buf);
> +
> + err = __set_inj(buf, cnt - 1);
> + if (err) {
> + pr_err("%s: Invalid flags value: %s\n", __func__, buf);
> + return err;
> + }
> +
> + *ppos += ret;
> +
> + return ret;
> +}
> +
> +static const struct file_operations flags_fops = {
> + .read = flags_read,
> + .write = flags_write,
> + .llseek = generic_file_llseek,
> +};
>
> /*
> * On which CPU to inject?
> @@ -130,7 +197,7 @@ static void do_inject(void)
> unsigned int cpu = i_mce.extcpu;
> u8 b = i_mce.bank;
>
> - if (!(i_mce.inject_flags & MCJ_EXCEPTION)) {
> + if (inj_type == SW_INJ) {
> amd_decode_mce(NULL, 0, &i_mce);
> return;
> }
> --
> 2.4.0
>
>
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
--
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/
Powered by blists - more mailing lists