On Tue, Nov 26, 2013 at 9:35 AM, Holger Brunck
<holger.brunck@xxxxxxxxxxx> wrote:
>
> this patch seems to break the support of a uio driver I have on 3.10-stable. I
> think I figured out whats the problem here.
[ commit 7314e613d5ff9f0934f7a0f74ed7973b903315d1 upstream ]
Uhhuh. No good. But:
>> + mem = idev->info->mem + mi;
>> +
>> + if (vma->vm_end - vma->vm_start > mem->size)
>> + return -EINVAL;
>>
>
> This check needs to be
> vma->vm_end - vma->vm_start < mem->size
No. The "vma->vm_end - vma->vm_start" is how big the virtual mmap size
is. So if you're trying to mmap a bigger area than your hardware
provides, we return -EINVAL. So the check in the patch is correct.
And the virtual mmap hole had better be *smaller* (or equal) to the
size of the hardware (well, "hardware") buffer size. Otherwise it
would contain pages from beyond the "mem->addr" array. So then the
user mapping can be used to modify random kernel pages outside the
allocated memory area. The technical term for that is "really not
good".
So your driver - or your users of your driver - seems to be broken.
They are mapping a bigger area than your driver is actually providing,
and you should fix that.
Alternatively, if there are users outside of your own setup for your
uio driver, we could possibly avoid the error by simply silently
refusing to map past the end of mem->size (and then take a SIGBUS if
somebody tries to access those pages).
But since this is the first report of breakage I have gotten, I'm
going to assume that the problem is limited enough that you can just
fix your uio driver. Of course, I don't actually know how widely used
the uio interface is, so the lack of reports may be more about "you're
the only user" than about "the code works fine for others" ;^p
Linus
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html