Thomas Graf <[email protected]> wrote:
>
> * Shailabh Nagar <[email protected]> 2006-07-06 07:37
> > @@ -37,9 +45,26 @@ static struct nla_policy taskstats_cmd_g
> > __read_mostly = {
> > [TASKSTATS_CMD_ATTR_PID] = { .type = NLA_U32 },
> > [TASKSTATS_CMD_ATTR_TGID] = { .type = NLA_U32 },
> > + [TASKSTATS_CMD_ATTR_REGISTER_CPUMASK] = { .type = NLA_STRING },
> > + [TASKSTATS_CMD_ATTR_DEREGISTER_CPUMASK] = { .type = NLA_STRING },};
>
> > + na = info->attrs[TASKSTATS_CMD_ATTR_REGISTER_CPUMASK];
> > + if (nla_len(na) > TASKSTATS_CPUMASK_MAXLEN)
> > + return -E2BIG;
> > + rc = cpulist_parse((char *)nla_data(na), mask);
>
> This isn't safe, the data in the attribute is not guaranteed to be
> NUL terminated. Still it's probably me to blame for not making
> this more obvious in the API.
>
Thanks, that was an unpleasant bug.
> I've attached a patch below extending the API to make it easier
> for interfaces using NUL termianted strings,
In the interests of keeping this work decoupled from netlink enhancements
I'd propose the below. Is it bad to modify the data at nla_data()?
--- a/kernel/taskstats.c~per-task-delay-accounting-taskstats-interface-control-exit-data-through-cpumasks-fix
+++ a/kernel/taskstats.c
@@ -299,6 +299,23 @@ cleanup:
return 0;
}
+static int parse(struct nlattr *na, cpumask_t *mask)
+{
+ char *data;
+ int len;
+
+ if (na == NULL)
+ return 1;
+ len = nla_len(na);
+ if (len > TASKSTATS_CPUMASK_MAXLEN)
+ return -E2BIG;
+ if (len < 1)
+ return -EINVAL;
+ data = nla_data(na);
+ data[len - 1] = '\0';
+ return cpulist_parse(data, *mask);
+}
+
static int taskstats_user_cmd(struct sk_buff *skb, struct genl_info *info)
{
int rc = 0;
@@ -309,27 +326,17 @@ static int taskstats_user_cmd(struct sk_
struct nlattr *na;
cpumask_t mask;
- if (info->attrs[TASKSTATS_CMD_ATTR_REGISTER_CPUMASK]) {
- na = info->attrs[TASKSTATS_CMD_ATTR_REGISTER_CPUMASK];
- if (nla_len(na) > TASKSTATS_CPUMASK_MAXLEN)
- return -E2BIG;
- rc = cpulist_parse((char *)nla_data(na), mask);
- if (rc)
- return rc;
- rc = add_del_listener(info->snd_pid, &mask, REGISTER);
+ rc = parse(info->attrs[TASKSTATS_CMD_ATTR_REGISTER_CPUMASK], &mask);
+ if (rc < 0)
return rc;
- }
+ if (rc == 0)
+ return add_del_listener(info->snd_pid, &mask, REGISTER);
- if (info->attrs[TASKSTATS_CMD_ATTR_DEREGISTER_CPUMASK]) {
- na = info->attrs[TASKSTATS_CMD_ATTR_DEREGISTER_CPUMASK];
- if (nla_len(na) > TASKSTATS_CPUMASK_MAXLEN)
- return -E2BIG;
- rc = cpulist_parse((char *)nla_data(na), mask);
- if (rc)
- return rc;
- rc = add_del_listener(info->snd_pid, &mask, DEREGISTER);
+ rc = parse(info->attrs[TASKSTATS_CMD_ATTR_DEREGISTER_CPUMASK], &mask);
+ if (rc < 0)
return rc;
- }
+ if (rc == 0)
+ return add_del_listener(info->snd_pid, &mask, DEREGISTER);
/*
* Size includes space for nested attributes
_
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
