Messages in this thread Patch in this message | | | Date | Thu, 6 Jul 2006 14:48:08 -0700 | From | Andrew Morton <> | Subject | Re: [PATCH] per-task delay accounting taskstats interface: control exit data through cpumasks] |
| |
Thomas Graf <tgraf@suug.ch> wrote: > > * Shailabh Nagar <nagar@watson.ibm.com> 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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |