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

Andrew Morton
2006-07-06T22:53:46Z

Shailabh Nagar <nagar@watson.ibm.com> wrote:
>
> Andrew Morton wrote:
> > 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.
>
> The patch looks good. I was also thinking of simply modifying the input
> to have the null termination and modify later when netlink provides
> generic support.
>
>

Yup. Thomas, what's the testing status of the netlink patch you sent? Should I
queue it up and start plagueing people with it?