| Subject | Re: [patch 05/11] syslets: core code | From | Andi Kleen <> | Date | 14 Feb 2007 00:15:58 +0100 |
| |
Ingo Molnar <mingo@elte.hu> writes:
> + > +static struct async_thread * > +pick_ready_cachemiss_thread(struct async_head *ah)
The cachemiss names are confusing. I assume that's just a left over from Tux? > + > + memset(atom->args, 0, sizeof(atom->args)); > + > + ret |= __get_user(arg_ptr, &uatom->arg_ptr[0]); > + if (!arg_ptr) > + return ret; > + if (!access_ok(VERIFY_WRITE, arg_ptr, sizeof(*arg_ptr))) > + return -EFAULT;
It's a little unclear why you do that many individual access_ok()s. And why is the target constant sized anyways?
+ /* + * Lock down the ring. Note: user-space should not munlock() this, + * because if the ring pages get swapped out then the async + * completion code might return a -EFAULT instead of the expected + * completion. (the kernel safely handles that case too, so this + * isnt a security problem.) + * + * mlock() is better here because it gets resource-accounted + * properly, and even unprivileged userspace has a few pages + * of mlock-able memory available. (which is more than enough + * for the completion-pointers ringbuffer) + */
If it's only a few pages you don't need any resource accounting. If it's more then it's nasty to steal the users quota. I think plain gup() would be better.
-Andi - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
|