Re: [PATCH] per-task delay accounting taskstats interface: control exit data through cpumasks]

Shailabh Nagar
2006-07-06T13:21:06Z

On Thu, 2006-07-06 at 14:08 +0200, Thomas Graf 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.
>
> I've attached a patch below extending the API to make it easier
> for interfaces using NUL termianted strings, so you'd do:
>
> [TASKSTATS_CMS_ATTR_REGISTER_CPUMASK] = { .type = NLA_NUL_STRING,
> .len = TASKSTATS_CPUMASK_MAXLEN }
>
> ... and then use (char *) nla_data(str_attr) without any further
> sanity checks.

Thanks. That makes it much clearer. I was looking around for a "maxlen"
attribute helper yesterday :-)

--Shailabh

>
> [NETLINK]: Improve string attribute validation
>
> Introduces a new attribute type NLA_NUL_STRING to support NUL
> terminated strings. Attributes of this kind require to carry
> a terminating NUL within the maximum specified in the policy.
>
> The `old' NLA_STRING which is not required to be NUL terminated
> is extended to provide means to specify a maximum length of the
> string.
>
> Aims at easing the pain with using nla_strlcpy() on temporary
> buffers.
>
> The old `minlen' field is renamed to `len' for cosmetic purposes
> which is ok since nobody was using it at this point.
>
> Signed-off-by: Thomas Graf <tgraf@suug.ch>

<snip>