[PATCH 0/4] devmem and readahead fixes for 2.6.33
Wu Fengguang
Here are some good fixes for 2.6.33, they have been floating around
with other patches for some time. I should really seperate them out
earlier..
Greg,
The first two patches are on devmem. 2.6.32 also needs fixing, however
the patches can only apply cleanly to 2.6.33. I can do backporting if
necessary.
[PATCH 1/4] devmem: check vmalloc address on kmem read/write
[PATCH 2/4] devmem: fix kmem write bug on memory holes
The next two patches are on readahead. All previous kernel needs fixing,
and the patches can apply cleanly to 2.6.32, too.
[PATCH 3/4] vfs: take f_lock on modifying f_mode after open time
[PATCH 4/4] readahead: introduce FMODE_RANDOM for POSIX_FADV_RANDOM
Thanks,
Fengguang
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Greg KH
> Andrew,
>
> Here are some good fixes for 2.6.33, they have been floating around
> with other patches for some time. I should really seperate them out
> earlier..
>
> Greg,
>
> The first two patches are on devmem. 2.6.32 also needs fixing, however
> the patches can only apply cleanly to 2.6.33. I can do backporting if
> necessary.
>
> [PATCH 1/4] devmem: check vmalloc address on kmem read/write
> [PATCH 2/4] devmem: fix kmem write bug on memory holes
After these hit Linus's tree, please send the backport to
sta...@kernel.org and I will be glad to queue them up.
thanks,
greg k-h
Andrew Morton
Greg KH <gre...@suse.de> wrote:
> On Fri, Jan 22, 2010 at 12:59:14PM +0800, Wu Fengguang wrote:
> > Andrew,
> >
> > Here are some good fixes for 2.6.33, they have been floating around
> > with other patches for some time. I should really seperate them out
> > earlier..
> >
> > Greg,
> >
> > The first two patches are on devmem. 2.6.32 also needs fixing, however
> > the patches can only apply cleanly to 2.6.33. I can do backporting if
> > necessary.
> >
> > [PATCH 1/4] devmem: check vmalloc address on kmem read/write
> > [PATCH 2/4] devmem: fix kmem write bug on memory holes
>
> After these hit Linus's tree, please send the backport to
> sta...@kernel.org and I will be glad to queue them up.
>
I tagged the first two patches for -stable and shall send them in for 2.6.33.
The second two patches aren't quite as obvious - perhaps a risk of
weird regressions. So I'm thinking I'll send them in for 2.6.34-rc1
and I tagged them as "[2.6.33.x]" for -stable, so you can feed them
into 2.6.33.x once 2.6.34-rcX has had a bit of testing time, OK?
Greg KH
> On Thu, 21 Jan 2010 21:31:57 -0800
> Greg KH <gre...@suse.de> wrote:
>
> > On Fri, Jan 22, 2010 at 12:59:14PM +0800, Wu Fengguang wrote:
> > > Andrew,
> > >
> > > Here are some good fixes for 2.6.33, they have been floating around
> > > with other patches for some time. I should really seperate them out
> > > earlier..
> > >
> > > Greg,
> > >
> > > The first two patches are on devmem. 2.6.32 also needs fixing, however
> > > the patches can only apply cleanly to 2.6.33. I can do backporting if
> > > necessary.
> > >
> > > [PATCH 1/4] devmem: check vmalloc address on kmem read/write
> > > [PATCH 2/4] devmem: fix kmem write bug on memory holes
> >
> > After these hit Linus's tree, please send the backport to
> > sta...@kernel.org and I will be glad to queue them up.
> >
>
> I tagged the first two patches for -stable and shall send them in for 2.6.33.
>
> The second two patches aren't quite as obvious - perhaps a risk of
> weird regressions. So I'm thinking I'll send them in for 2.6.34-rc1
> and I tagged them as "[2.6.33.x]" for -stable, so you can feed them
> into 2.6.33.x once 2.6.34-rcX has had a bit of testing time, OK?
Sounds good to me.
thanks,
greg k-h
Wu Fengguang
> On Thu, 21 Jan 2010 21:31:57 -0800
> Greg KH <gre...@suse.de> wrote:
>
> > On Fri, Jan 22, 2010 at 12:59:14PM +0800, Wu Fengguang wrote:
> > > Andrew,
> > >
> > > Here are some good fixes for 2.6.33, they have been floating around
> > > with other patches for some time. I should really seperate them out
> > > earlier..
> > >
> > > Greg,
> > >
> > > The first two patches are on devmem. 2.6.32 also needs fixing, however
> > > the patches can only apply cleanly to 2.6.33. I can do backporting if
> > > necessary.
> > >
> > > [PATCH 1/4] devmem: check vmalloc address on kmem read/write
> > > [PATCH 2/4] devmem: fix kmem write bug on memory holes
> >
> > After these hit Linus's tree, please send the backport to
> > sta...@kernel.org and I will be glad to queue them up.
> >
>
> I tagged the first two patches for -stable and shall send them in for 2.6.33.
>
> The second two patches aren't quite as obvious - perhaps a risk of
> weird regressions. So I'm thinking I'll send them in for 2.6.34-rc1
> and I tagged them as "[2.6.33.x]" for -stable, so you can feed them
> into 2.6.33.x once 2.6.34-rcX has had a bit of testing time, OK?
OK, I'll send the patches to stable kernel once they hit mainline.
Thanks,
Fengguang
Greg KH
>
> The first two patches are on devmem. 2.6.32 also needs fixing, however
> the patches can only apply cleanly to 2.6.33. I can do backporting if
> necessary.
>
> [PATCH 1/4] devmem: check vmalloc address on kmem read/write
> [PATCH 2/4] devmem: fix kmem write bug on memory holes
As these patches are now in Linus's tree, can you provide backports for
them and send them to the sta...@kernel.org address?
thanks,
greg k-h
Wu Fengguang
commit 325fda71d0badc1073dc59f12a948f24ff05796a upstream.
Otherwise vmalloc_to_page() will BUG().
This also makes the kmem read/write implementation aligned with mem(4):
"References to nonexistent locations cause errors to be returned." Here
we return -ENXIO (inspired by Hugh) if no bytes have been transfered
to/from user space, otherwise return partial read/write results.
CC: Greg Kroah-Hartman <gre...@suse.de>
CC: Hugh Dickins <hugh.d...@tiscali.co.uk>
CC: <sta...@kernel.org>
Signed-off-by: KAMEZAWA Hiroyuki <kamezaw...@jp.fujitsu.com>
Signed-off-by: Wu Fengguang <fenggu...@intel.com>
---
drivers/char/mem.c | 28 ++++++++++++++++++----------
1 file changed, 18 insertions(+), 10 deletions(-)
--- linux-2.6.32.orig/drivers/char/mem.c 2010-02-04 10:28:19.000000000 +0800
+++ linux-2.6.32/drivers/char/mem.c 2010-02-04 10:37:55.000000000 +0800
@@ -408,6 +408,7 @@ static ssize_t read_kmem(struct file *fi
unsigned long p = *ppos;
ssize_t low_count, read, sz;
char * kbuf; /* k-addr because vread() takes vmlist_lock rwlock */
+ int err = 0;
read = 0;
if (p < (unsigned long) high_memory) {
@@ -464,14 +465,18 @@ static ssize_t read_kmem(struct file *fi
while (count > 0) {
int len = count;
+ if (!is_vmalloc_or_module_addr((void *)p)) {
+ err = -ENXIO;
+ break;
+ }
if (len > PAGE_SIZE)
len = PAGE_SIZE;
len = vread(kbuf, (char *)p, len);
if (!len)
break;
if (copy_to_user(buf, kbuf, len)) {
- free_page((unsigned long)kbuf);
- return -EFAULT;
+ err = -EFAULT;
+ break;
}
count -= len;
buf += len;
@@ -480,8 +485,8 @@ static ssize_t read_kmem(struct file *fi
}
free_page((unsigned long)kbuf);
}
- *ppos = p;
- return read;
+ *ppos = p;
+ return read ? read : err;
}
@@ -557,6 +562,7 @@ static ssize_t write_kmem(struct file *
ssize_t virtr = 0;
ssize_t written;
char * kbuf; /* k-addr because vwrite() takes vmlist_lock rwlock */
+ int err = 0;
if (p < (unsigned long) high_memory) {
@@ -580,15 +586,17 @@ static ssize_t write_kmem(struct file *
while (count > 0) {
int len = count;
+ if (!is_vmalloc_or_module_addr((void *)p)) {
+ err = -ENXIO;
+ break;
+ }
if (len > PAGE_SIZE)
len = PAGE_SIZE;
if (len) {
written = copy_from_user(kbuf, buf, len);
if (written) {
- if (wrote + virtr)
- break;
- free_page((unsigned long)kbuf);
- return -EFAULT;
+ err = -EFAULT;
+ break;
}
}
len = vwrite(kbuf, (char *)p, len);
@@ -600,8 +608,8 @@ static ssize_t write_kmem(struct file *
free_page((unsigned long)kbuf);
}
- *ppos = p;
- return virtr + wrote;
+ *ppos = p;
+ return virtr + wrote ? : err;
}
#endif
KAMEZAWA Hiroyuki
Wu Fengguang <fenggu...@intel.com> wrote:
> From: KAMEZAWA Hiroyuki <kamezaw...@jp.fujitsu.com>
>
> commit 325fda71d0badc1073dc59f12a948f24ff05796a upstream.
>
> Otherwise vmalloc_to_page() will BUG().
>
> This also makes the kmem read/write implementation aligned with mem(4):
> "References to nonexistent locations cause errors to be returned." Here
> we return -ENXIO (inspired by Hugh) if no bytes have been transfered
> to/from user space, otherwise return partial read/write results.
>
Wu-san, I have additonal fix to this patch. Now, *ppos update is unstable..
Could you make merged one ?
Maybe this one makes the all behavior clearer.
==
This is a more fix for devmem-check-vmalloc-address-on-kmem-read-write.patch
Now, the condition for updating *ppos is not good. (it's updated even if EFAULT
occurs..). This fixes that.
Reported-by: "Juha Leppanen" <juha_moto...@luukku.com>
CC: Wu Fengguang <fenggu...@intel.com>
Signed-off-by: KAMEZAWA Hiroyuki <kamezaw...@jp.fujitsu.com>
---
drivers/char/mem.c | 34 +++++++++++++++++++++++++---------
1 file changed, 25 insertions(+), 9 deletions(-)
Index: mmotm-2.6.33-Feb01/drivers/char/mem.c
===================================================================
--- mmotm-2.6.33-Feb01.orig/drivers/char/mem.c
+++ mmotm-2.6.33-Feb01/drivers/char/mem.c
@@ -460,14 +460,18 @@ static ssize_t read_kmem(struct file *fi
}
free_page((unsigned long)kbuf);
}
+ /* EFAULT is always critical */
+ if (err == -EFAULT)
+ return err;
+ if (err == -ENXIO && !read)
+ return -ENXIO;
*ppos = p;
- return read ? read : err;
+ return read;
}
static inline ssize_t
-do_write_kmem(unsigned long p, const char __user *buf,
- size_t count, loff_t *ppos)
+do_write_kmem(unsigned long p, const char __user *buf, size_t count)
{
ssize_t written, sz;
unsigned long copied;
@@ -510,7 +514,6 @@ do_write_kmem(unsigned long p, const cha
written += sz;
}
- *ppos += written;
return written;
}
@@ -521,6 +524,7 @@ do_write_kmem(unsigned long p, const cha
static ssize_t write_kmem(struct file * file, const char __user * buf,
size_t count, loff_t *ppos)
{
+ /* Kernel virtual memory never exceeds unsigned long */
unsigned long p = *ppos;
ssize_t wrote = 0;
ssize_t virtr = 0;
@@ -530,7 +534,7 @@ static ssize_t write_kmem(struct file *
if (p < (unsigned long) high_memory) {
unsigned long to_write = min_t(unsigned long, count,
(unsigned long)high_memory - p);
- wrote = do_write_kmem(p, buf, to_write, ppos);
+ wrote = do_write_kmem(p, buf, to_write);
if (wrote != to_write)
return wrote;
p += wrote;
@@ -540,8 +544,13 @@ static ssize_t write_kmem(struct file *
if (count > 0) {
kbuf = (char *)__get_free_page(GFP_KERNEL);
- if (!kbuf)
- return wrote ? wrote : -ENOMEM;
+ if (!kbuf) {
+ if (wrote) { /* update ppos and return copied bytes */
+ *ppos = p;
+ return wrote;
+ } else
+ return -ENOMEM;
+ }
while (count > 0) {
unsigned long sz = size_inside_page(p, count);
unsigned long n;
@@ -563,9 +572,16 @@ static ssize_t write_kmem(struct file *
}
free_page((unsigned long)kbuf);
}
-
+ /* EFAULT is always critical. */
+ if (err == -EFAULT)
+ return err;
+ if (err == -ENXIO) {
+ /* We reached the end of vmalloc area..check real bug or not*/
+ if (!(virtr + wrote)) /* nothing written */
+ return -ENXIO;
+ }
*ppos = p;
- return virtr + wrote ? : err;
+ return virtr + wrote;
Wu Fengguang
commit c85e9a97c4102ce2e83112da850d838cfab5ab13 upstream.
write_kmem() used to assume vwrite() always return the full buffer length.
However now vwrite() could return 0 to indicate memory hole. This creates
a bug that "buf" is not advanced accordingly.
Fix it to simply ignore the return value, hence the memory hole.
CC: Andi Kleen <an...@firstfloor.org>
CC: Benjamin Herrenschmidt <be...@kernel.crashing.org>
CC: Christoph Lameter <c...@linux-foundation.org>
CC: Ingo Molnar <mi...@elte.hu>
CC: Tejun Heo <t...@kernel.org>
CC: Nick Piggin <npi...@suse.de>
CC: KAMEZAWA Hiroyuki <kamezaw...@jp.fujitsu.com>
CC: <sta...@kernel.org>
Signed-off-by: Wu Fengguang <fenggu...@intel.com>
---
drivers/char/mem.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- linux-2.6.32.orig/drivers/char/mem.c 2010-02-04 10:37:55.000000000 +0800
+++ linux-2.6.32/drivers/char/mem.c 2010-02-04 10:37:59.000000000 +0800
@@ -599,7 +599,7 @@ static ssize_t write_kmem(struct file *
break;
}
}
- len = vwrite(kbuf, (char *)p, len);
+ vwrite(kbuf, (char *)p, len);
count -= len;
buf += len;
virtr += len;
Wu Fengguang
> On Thu, 4 Feb 2010 10:42:02 +0800
> Wu Fengguang <fenggu...@intel.com> wrote:
>
> > From: KAMEZAWA Hiroyuki <kamezaw...@jp.fujitsu.com>
> >
> > commit 325fda71d0badc1073dc59f12a948f24ff05796a upstream.
> >
> > Otherwise vmalloc_to_page() will BUG().
> >
> > This also makes the kmem read/write implementation aligned with mem(4):
> > "References to nonexistent locations cause errors to be returned." Here
> > we return -ENXIO (inspired by Hugh) if no bytes have been transfered
> > to/from user space, otherwise return partial read/write results.
> >
>
> Wu-san, I have additonal fix to this patch. Now, *ppos update is unstable..
> Could you make merged one ?
> Maybe this one makes the all behavior clearer.
>
> ==
> This is a more fix for devmem-check-vmalloc-address-on-kmem-read-write.patch
> Now, the condition for updating *ppos is not good. (it's updated even if EFAULT
> occurs..). This fixes that.
>
>
> Reported-by: "Juha Leppanen" <juha_moto...@luukku.com>
Sorry, can you elaborate the problem? How it break the application?
It looks that do_generic_file_read() also updates *ppos progressively,
no one complains about that.
Thanks,
Fengguang
KAMEZAWA Hiroyuki
Wu Fengguang <fenggu...@intel.com> wrote:
> On Thu, Feb 04, 2010 at 10:58:01AM +0800, KAMEZAWA Hiroyuki wrote:
> > On Thu, 4 Feb 2010 10:42:02 +0800
> > Wu Fengguang <fenggu...@intel.com> wrote:
> >
> > > From: KAMEZAWA Hiroyuki <kamezaw...@jp.fujitsu.com>
> > >
> > > commit 325fda71d0badc1073dc59f12a948f24ff05796a upstream.
> > >
> > > Otherwise vmalloc_to_page() will BUG().
> > >
> > > This also makes the kmem read/write implementation aligned with mem(4):
> > > "References to nonexistent locations cause errors to be returned." Here
> > > we return -ENXIO (inspired by Hugh) if no bytes have been transfered
> > > to/from user space, otherwise return partial read/write results.
> > >
> >
> > Wu-san, I have additonal fix to this patch. Now, *ppos update is unstable..
> > Could you make merged one ?
> > Maybe this one makes the all behavior clearer.
> >
> > ==
> > This is a more fix for devmem-check-vmalloc-address-on-kmem-read-write.patch
> > Now, the condition for updating *ppos is not good. (it's updated even if EFAULT
> > occurs..). This fixes that.
> >
> >
> > Reported-by: "Juha Leppanen" <juha_moto...@luukku.com>
>
> Sorry, can you elaborate the problem? How it break the application?
>
> It looks that do_generic_file_read() also updates *ppos progressively,
> no one complains about that.
>
Ah...it seems I misunderstood something...ok, *ppos should be updated every time.
I startted from adding comment on following line and got into a maze.
> return (virtr + wrote) ? : err;
Sorry for noise.
-Kame