lkml.org 
[lkml]   [2010]   [Jul]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] arch/tile: updates to hardwall code from community feedback.
On Fri, Jul 02, 2010 at 11:45:18AM -0400, Chris Metcalf wrote:
> #define for_each_hardwall_task_safe(pos, n, head) \
> - hardwall_for_each_entry_safe(pos, n, head, thread.hardwall_list)
> + list_for_each_entry_safe(pos, n, head, thread.hardwall_list)
>
If you've killed off the rest of the wrappers due to them not really
having to do anything special, you could do the same for this one, too.

> static struct hardwall_info *hardwall_create(
> - size_t size, const unsigned long __user *bits)
> + size_t size, const unsigned char __user *bits)
> {
> struct hardwall_info *iter, *rect;
> struct cpumask mask;
> unsigned long flags;
> - int rc, cpu, maxcpus = smp_height * smp_width;
> + int rc;
>
> - /* If "size" is not a multiple of long, it's invalid. */
> - if (size % sizeof(long))
> + /* Reject crazy sizes out of hand, a la sys_mbind(). */
> + if (size > PAGE_SIZE)
> return ERR_PTR(-EINVAL);
>
Does it even make any sense to accept > sizeof(struct cpumask) ? Your
case below obviously handles this by making sure the rest of the bits are
zeroed, but that still seems a bit excessive. If anything, the
sizeof(struct cpumask) should be your worst case (or just rejected out of
hand), and you could use cpumask_size() for everything else.

> - /* Validate that all the bits we are given fit in our coordinates. */
> - cpumask_clear(&mask);
> - for (cpu = 0; size > 0; ++bits, size -= sizeof(long)) {
> - int i;
> - unsigned long m;
> - if (get_user(m, bits) != 0)
> - return ERR_PTR(-EFAULT);
> - for (i = 0; i < BITS_PER_LONG; ++i, ++cpu)
> - if (m & (1UL << i)) {
> - if (cpu >= maxcpus)
> - return ERR_PTR(-EINVAL);
> - cpumask_set_cpu(cpu, &mask);
> - }
> + /* Copy whatever fits into a cpumask. */
> + if (copy_from_user(&mask, bits, min(sizeof(struct cpumask), size)))
> + return ERR_PTR(-EFAULT);
> +
> + /*
> + * If the size was short, clear the rest of the mask;
> + * otherwise validate that the rest of the user mask was zero
> + * (we don't try hard to be efficient when validating huge masks).
> + */
> + if (size < sizeof(struct cpumask)) {
> + memset((char *)&mask + size, 0, sizeof(struct cpumask) - size);
> + } else if (size > sizeof(struct cpumask)) {
> + size_t i;
> + for (i = sizeof(struct cpumask); i < size; ++i) {
> + char c;
> + if (get_user(c, &bits[i]))
> + return ERR_PTR(-EFAULT);
> + if (c)
> + return ERR_PTR(-EINVAL);
> + }
> }
>
Surely you can just replace all of this with cpumask_parse_user()?


\
 
 \ /
  Last update: 2010-07-04 06:01    [W:0.085 / U:0.056 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site