Re: [PATCH 4/4] MSR: msr Batch processing feature

[One Thousand Gnomes]

> +	myinfo->rawio_allowed = capable(CAP_SYS_RAWIO);
> +	file->private_data = myinfo;

That strikes me as a very bad idea btw. If your opener was privileged and
leaks the handle via exec or anything else to another process that
process inherits the powers which means it can own the system trivially.

(Texbook example was the early 4BSD socket bug where any root created
socket could be used to reconfigure networking which meant rsh could be
used to do wonderful things. Then faithfully duplicated in Solaris 2.x
*after* BSD fixed it ;) )

> +static long msrbatch_ioctl(struct file *f, unsigned int ioc, unsigned long arg)
> +{
> +	int err = 0;
> +	struct msr_batch_array __user *uoa;
> +	struct msr_batch_op __user *uops;
> +	struct msr_batch_array koa;
> +	struct msrbatch_session_info *myinfo = f->private_data;
> +
> +	if (ioc != X86_IOC_MSR_BATCH) {
> +		pr_err("Invalid ioctl op %u\n", ioc);

So a user can fill your log because you have lots of pr_err() calls etc

> +	err = msrbatch_apply_whitelist(&koa, myinfo);

Two threads doing this at once will break if they issue overlapping
requests with and/or (plus whatever carnage if you clash with any other
kernel used MSR)

Alan