Thomas Graf <[EMAIL PROTECTED]> wrote:
>
> * Andrew Morton <[EMAIL PROTECTED]> 2006-07-06 14:48
> > 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()?
> 
> Yes, it points into a skb data buffer which may be shared by sitting
> on other queues if the message is to be broadcasted. In this case it
> would be harmless but the policy is to leave it unmodified.

Yup, sleazy-but-safe.

> 
> > --- 
> > 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);
> > +}
> 
> Usually this is done by using strlcpy:

hm, nla_strlcpy() looks more complex than it needs to be.  We really need
nla_kstrndup() ;)

Oh well.  This?


diff -puN 
kernel/taskstats.c~per-task-delay-accounting-taskstats-interface-control-exit-data-through-cpumasks-fix
 kernel/taskstats.c
--- 
a/kernel/taskstats.c~per-task-delay-accounting-taskstats-interface-control-exit-data-through-cpumasks-fix
+++ a/kernel/taskstats.c
@@ -299,6 +299,28 @@ cleanup:
        return 0;
 }
 
+static int parse(struct nlattr *na, cpumask_t *mask)
+{
+       char *data;
+       int len;
+       int ret;
+
+       if (na == NULL)
+               return 1;
+       len = nla_len(na);
+       if (len > TASKSTATS_CPUMASK_MAXLEN)
+               return -E2BIG;
+       if (len < 1)
+               return -EINVAL;
+       data = kmalloc(len, GFP_KERNEL);
+       if (!data)
+               return -ENOMEM;
+       nla_strlcpy(data, na, len);
+       ret = cpulist_parse(data, *mask);
+       kfree(data);
+       return ret;
+}
+
 static int taskstats_user_cmd(struct sk_buff *skb, struct genl_info *info)
 {
        int rc = 0;
@@ -309,27 +331,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 netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to