Hi, Paul,
I've just started playing with the new patchset and found a few issues.
[email protected] wrote:
> +ssize_t res_group_file_write(struct container *cont,
> + struct cftype *cft,
> + struct file *file,
> + const char __user *userbuf,
> + size_t nbytes, loff_t *ppos)
> +{
> + struct res_group_cft *rgcft = container_of(cft, struct res_group_cft, cft);
> + struct res_controller *ctlr = rgcft->ctlr;
> +
> + if (nbytes >= PAGE_SIZE)
> + return -E2BIG;
> +
> + char *buf;
> + ssize_t retval;
> + int filetype = cft->private;
> +
> + buf = kmalloc(nbytes + 1, GFP_USER);
This should be kmalloc(nbytes), an echo ".." has a "\n" associated
with it.
> + if (!buf) return -ENOMEM;
> + if (copy_from_user(buf, userbuf, nbytes)) {
> + retval = -EFAULT;
> + goto out1;
> + }
> + buf[nbytes] = 0; /* nul-terminate */
> +
this should be buf[nbytes - 1]
--
Balbir Singh,
Linux Technology Center,
IBM Software Labs
Balbir wrote:
> This should be kmalloc(nbytes), an echo ".." has a "\n" associated
> with it.
But a:
write(1, "..", 2);
does not have a trialing newline.
If some consumer of this kernel buffer copy of what the
user wrote cannot handle the possible trailing whitespace,
they will have to chomp (Perl phrase) it off. You can't
just whack one byte blindly.
At least for the kernel/cpuset.c code, from whence this
came, the consumers of this kernel buffer copy are such
routines as simple_strtoul() and cpulist_parse(), both
of which cope with trailing newlines.
--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <[email protected]> 1.925.600.0401
Paul Jackson wrote:
> Balbir wrote:
>> This should be kmalloc(nbytes), an echo ".." has a "\n" associated
>> with it.
>
> But a:
> write(1, "..", 2);
> does not have a trialing newline.
Yes, true.
>
> If some consumer of this kernel buffer copy of what the
> user wrote cannot handle the possible trailing whitespace,
> they will have to chomp (Perl phrase) it off. You can't
> just whack one byte blindly.
>
Yes, agreed.
> At least for the kernel/cpuset.c code, from whence this
> came, the consumers of this kernel buffer copy are such
> routines as simple_strtoul() and cpulist_parse(), both
> of which cope with trailing newlines.
>
The problem I have is that match_token() that's used by
the resource group's infrastructure cannot deal with
"\n". I think the code needs in res_groups needs to
get smarter like the code in simple_strtoul()
--
Thanks for the feedback,
Balbir Singh,
Linux Technology Center,
IBM Software Labs
Paul Jackson wrote:
> Balbir wrote:
>> This should be kmalloc(nbytes), an echo ".." has a "\n" associated
>> with it.
>
> But a:
> write(1, "..", 2);
> does not have a trialing newline.
Yes, true.
>
> If some consumer of this kernel buffer copy of what the
> user wrote cannot handle the possible trailing whitespace,
> they will have to chomp (Perl phrase) it off. You can't
> just whack one byte blindly.
>
Yes, agreed.
> At least for the kernel/cpuset.c code, from whence this
> came, the consumers of this kernel buffer copy are such
> routines as simple_strtoul() and cpulist_parse(), both
> of which cope with trailing newlines.
>
The problem I have is that match_token() that's used by
the resource group's infrastructure cannot deal with
"\n". I think the code needs in res_groups needs to
get smarter like the code in simple_strtoul()
--
Thanks for the feedback,
Balbir Singh,
Linux Technology Center,
IBM Software Labs