Re: Question: how to handle too big f_pos Re: [PATCH] devmem: handlepartial kmem write/read

From: Hugh Dickins
Date: Tue Sep 15 2009 - 05:53:07 EST


On Tue, 15 Sep 2009, KAMEZAWA Hiroyuki wrote:

> I'm writing a patch against /dev/kmem...I found a problem.
>
> /dev/kmem (and /proc/<pid>/mem) puts virtual addres to f->f_pos.
>
> And, f->f_pos is loff_t .... signed value (not unsigned)
>
> Then, this check
> rw_verify_area(READ, file, pos, count);
> =>
> pos = *ppos;
> if (unlikely((pos < 0) || (loff_t) (pos + count) < 0))
> return retval;
>
> always returns -EINVAL when I read /dev/kmem's valid address.
>
> Then, how should I do for read /dev/kmem ? Any idea ?
> (Added Al Viro to CC:)

I believe what's supposed to happen is that special drivers like
/dev/mem and /dev/kmem provide their own .llseek method (they do).
But at some point along the way rw_verify_area() got "improved"
and now prevents them working on many configurations.

I noticed around 2.6.24-rc1, and have been building my debug kernels
with the patch below ever since then, as I sometimes like to peek and
poke into /dev/kmem to examine and try different things while running.

But whether it's the right patch is another matter. Though this should
be independent of that, I've also got a patch at the drivers/char/mem.c
end (I'll post that shortly in the main thread, not appropriate here),
and have never found time to consider whether these hacks amount to
anything more than "works for me".

IIRC, this patch below only covers one aspect of the negative offset
problem. Where the problems lie does change from time to time - back
in 2003 I used to use pread() and pwrite() to avoid llseek() issues;
but when I came back to it in 2007, I think I found those no longer
worked (on i386 or x86_64 or powerpc?), and lseek64() was the best bet.

Anyway, for what it's worth, ...

--- 2.6.31/fs/read_write.c 2009-09-09 23:13:59.000000000 +0100
+++ 2.6.31d/fs/read_write.c 2009-09-10 09:38:30.000000000 +0100
@@ -222,8 +222,13 @@ int rw_verify_area(int read_write, struc
if (unlikely((ssize_t) count < 0))
return retval;
pos = *ppos;
- if (unlikely((pos < 0) || (loff_t) (pos + count) < 0))
- return retval;
+ if (pos >= 0) {
+ if (unlikely((loff_t) (pos + count) < 0))
+ count = 1 + (size_t) LLONG_MAX - pos;
+ } else {
+ if (unlikely((loff_t) (pos + count) > 0))
+ count = - pos;
+ }

if (unlikely(inode->i_flock && mandatory_lock(inode))) {
retval = locks_mandatory_area(
--
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/