Adrian Bunk wrote:
> We'd better not nlmsg_free on a pointer containing an undefined value
> (and without having anything allocated)...
>
> Spotted by the Coverity checker.
>
> Signed-off-by: Adrian Bunk <[EMAIL PROTECTED]>
>
> ---
>
> kernel/taskstats.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> --- linux-2.6/kernel/taskstats.c.old 2007-10-23 19:01:07.000000000 +0200
> +++ linux-2.6/kernel/taskstats.c 2007-10-23 19:21:54.000000000 +0200
> @@ -383,67 +383,67 @@
>
> static int cgroupstats_user_cmd(struct sk_buff *skb, struct genl_info *info)
> {
> int rc = 0;
> struct sk_buff *rep_skb;
> struct cgroupstats *stats;
> struct nlattr *na;
> size_t size;
> u32 fd;
> struct file *file;
> int fput_needed;
>
> na = info->attrs[CGROUPSTATS_CMD_ATTR_FD];
> if (!na)
> return -EINVAL;
>
> fd = nla_get_u32(info->attrs[CGROUPSTATS_CMD_ATTR_FD]);
> file = fget_light(fd, &fput_needed);
> if (file) {
> size = nla_total_size(sizeof(struct cgroupstats));
>
> rc = prepare_reply(info, CGROUPSTATS_CMD_NEW, &rep_skb,
> size);
> if (rc < 0)
> - goto err;
> + return rc;
>
We miss a fput_light() here
> na = nla_reserve(rep_skb, CGROUPSTATS_TYPE_CGROUP_STATS,
> sizeof(struct cgroupstats));
> stats = nla_data(na);
> memset(stats, 0, sizeof(*stats));
>
> rc = cgroupstats_build(stats, file->f_dentry);
> +
> + fput_light(file, fput_needed);
> +
I don't understand this movement, it makes code reading a bit odd too.
rc is the result, but we check the result after fput_light?
> if (rc < 0)
> goto err;
>
> - fput_light(file, fput_needed);
> return send_reply(rep_skb, info->snd_pid);
> +err:
> + nlmsg_free(rep_skb);
> }
>
> -err:
> - if (file)
> - fput_light(file, fput_needed);
> - nlmsg_free(rep_skb);
> return rc;
> }
--
Warm Regards,
Balbir Singh
Linux Technology Center
IBM, ISTL
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
