Re: [GIT PULL] char/misc driver patches for 3.16-rc1

[Greg KH]

On Tue, Jun 03, 2014 at 08:32:50AM -0700, Linus Torvalds wrote:
> On Mon, Jun 2, 2014 at 10:44 PM, Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> >
> > Bin Wang (1):
> >       uio: fix vma io range check in mmap
> 
> Greg, this is BS.
> 
> If the UIO memory size is smaller than a page, we cannot mmap it
> safely, because the mmap will map random memory *after* the memory
> area too. This is not like a regular file mapping where the kernel can
> just zero-pad up to the end of the page.
> 
> We had this bug before (and even worse - it would mmap unaligned IO
> structures too, so now the actual mapped address didn't actually
> correspond to the returned user mapping address at all), and we fixed
> them. See
> 
>   7314e613d5ff Fix a few incorrectly checked [io_]remap_pfn_range() calls
>   b65502879556 uio: we cannot mmap unaligned page contents
> 
> so now you've re-introduced part of the problem, and marked it for stable too.

Crap, sorry about that, I'll remember to not take it for stable.

> The commit log shows nothing useful. It basically just says "let's
> reintroduce this bug" without even giving an excuse why that would be
> a good idea.
> 
> And it really _isn't_ a good idea. At least you didn't remove the
> alignment check, but the thing is, if a resource is less than a page
> in size, it's quite possibly also unaligned, so the fix doesn't even
> *fix* anything, except by pure luck. The fact is, memory-mapping
> device areas smaller than one page is simply a bad bad idea.
> 
> Don't do this shit.

Hm, I got two different bug reports, and this same patch from two
different people insisting that we broke their drivers with the above
patches, and asked for this patch to be applied.  Bin provided a big
long description of why this change was acceptable, somewhere in the
lkml archives, and I got confused thinking it was ok as long as the size
was aligned properly, my fault, I didn't think about the trailing data
in the buffer :(

Bin and Nobuhiro, you need to look at your userspace program that is
dealing with the uio interface and fix up the numbers you are passing it
to live with the existing changes.

Linus, I'll send you a revert patch for this now, or do you want to just
do it directly?

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/