public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Greg KH <gregkh@linuxfoundation.org>
To: Linus Torvalds <torvalds@linux-foundation.org>,
	Bin Wang <binw@marvell.com>,
	Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj@renesas.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Arnd Bergmann <arnd@arndb.de>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [GIT PULL] char/misc driver patches for 3.16-rc1
Date: Tue, 3 Jun 2014 10:02:44 -0700	[thread overview]
Message-ID: <20140603170244.GA22867@kroah.com> (raw)
In-Reply-To: <CA+55aFyQ=iSMnLAywumYpGBOBiqxL2qO9u9OsTF=RFfYBnz10g@mail.gmail.com>

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@linuxfoundation.org> 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

  reply	other threads:[~2014-06-03 16:59 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-03  5:44 [GIT PULL] char/misc driver patches for 3.16-rc1 Greg KH
2014-06-03 15:32 ` Linus Torvalds
2014-06-03 17:02   ` Greg KH [this message]
2014-06-03 17:14     ` Linus Torvalds
2014-06-03 18:25       ` Greg KH
2014-06-07 10:41         ` Pavel Machek
2014-06-07 10:42         ` Pavel Machek
2014-06-07 22:02           ` One Thousand Gnomes
2014-06-17 23:06         ` Greg KH

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140603170244.GA22867@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=akpm@linux-foundation.org \
    --cc=arnd@arndb.de \
    --cc=binw@marvell.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nobuhiro.iwamatsu.yj@renesas.com \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox