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

From: Andrew Morton <[email-masked]>
Date: Thu, 6 Jul 2006 15:56:43 -0700

Shailabh Nagar <[email-masked]> wrote:
>
> Andrew Morton wrote:
> > Thomas Graf <[email-masked]> wrote:
> > 
> >>* Shailabh Nagar <[email-masked]> 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?