* [PATCH] /dev/mem: Fix wrong error on accessing beyond valid memory addresses.
@ 2011-11-09 23:31 Andrei Warkentin
2011-11-14 21:21 ` Andrei Warkentin
0 siblings, 1 reply; 6+ messages in thread
From: Andrei Warkentin @ 2011-11-09 23:31 UTC (permalink / raw)
To: linux-kernel; +Cc: Andrei Warkentin, H. Peter Anvin
Currently this returns -EFAULT, but it really should be returning 0,
as in - 0 bytes read or written. This is what you would get by
opening a block device, seeking to the end, and trying to read
something. Additionally, make lseek() check the sought-to offset
to pass the valid_phys_addr_range test.
Cc: H. Peter Anvin <hpa@zytor.com>
Signed-off-by: Andrei Warkentin <andreiw@vmware.com>
---
drivers/char/mem.c | 9 +++++++--
1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/char/mem.c b/drivers/char/mem.c
index 8fc04b4..02d0b1a 100644
--- a/drivers/char/mem.c
+++ b/drivers/char/mem.c
@@ -98,7 +98,7 @@ static ssize_t read_mem(struct file *file, char __user *buf,
char *ptr;
if (!valid_phys_addr_range(p, count))
- return -EFAULT;
+ return 0;
read = 0;
#ifdef __ARCH_HAS_NO_PAGE_ZERO_MAPPED
/* we don't have page 0 mapped on sparc and m68k.. */
@@ -156,7 +156,7 @@ static ssize_t write_mem(struct file *file, const char __user *buf,
void *ptr;
if (!valid_phys_addr_range(p, count))
- return -EFAULT;
+ return 0;
written = 0;
@@ -710,6 +710,11 @@ static loff_t memory_lseek(struct file *file, loff_t offset, int orig)
case SEEK_CUR:
offset += file->f_pos;
case SEEK_SET:
+ if (!valid_phys_addr_range(offset, 0)) {
+ ret = -EINVAL;
+ break;
+ }
+
/* to avoid userland mistaking f_pos=-9 as -EBADF=-9 */
if ((unsigned long long)offset >= ~0xFFFULL) {
ret = -EOVERFLOW;
--
1.7.4.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] /dev/mem: Fix wrong error on accessing beyond valid memory addresses.
2011-11-09 23:31 [PATCH] /dev/mem: Fix wrong error on accessing beyond valid memory addresses Andrei Warkentin
@ 2011-11-14 21:21 ` Andrei Warkentin
2011-11-14 21:59 ` H. Peter Anvin
0 siblings, 1 reply; 6+ messages in thread
From: Andrei Warkentin @ 2011-11-14 21:21 UTC (permalink / raw)
To: H. Peter Anvin; +Cc: linux-kernel
Hi Peter,
----- Original Message -----
> From: "Andrei Warkentin" <andreiw@vmware.com>
> To: linux-kernel@vger.kernel.org
> Cc: "Andrei Warkentin" <andreiw@vmware.com>, "H. Peter Anvin" <hpa@zytor.com>
> Sent: Wednesday, November 9, 2011 6:31:19 PM
> Subject: [PATCH] /dev/mem: Fix wrong error on accessing beyond valid memory addresses.
>
> Currently this returns -EFAULT, but it really should be returning 0,
> as in - 0 bytes read or written. This is what you would get by
> opening a block device, seeking to the end, and trying to read
> something. Additionally, make lseek() check the sought-to offset
> to pass the valid_phys_addr_range test.
>
> Cc: H. Peter Anvin <hpa@zytor.com>
> Signed-off-by: Andrei Warkentin <andreiw@vmware.com>
> ---
I know you're very busy, but I was wondering if you could
give your two cents on this change. This change grew from
our discussion on LKML last week.
Thanks ahead,
A
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] /dev/mem: Fix wrong error on accessing beyond valid memory addresses.
2011-11-14 21:21 ` Andrei Warkentin
@ 2011-11-14 21:59 ` H. Peter Anvin
2011-11-14 22:11 ` Andrei Warkentin
0 siblings, 1 reply; 6+ messages in thread
From: H. Peter Anvin @ 2011-11-14 21:59 UTC (permalink / raw)
To: Andrei Warkentin; +Cc: linux-kernel
On 11/14/2011 01:21 PM, Andrei Warkentin wrote:
>
> I know you're very busy, but I was wondering if you could
> give your two cents on this change. This change grew from
> our discussion on LKML last week.
>
> Thanks ahead,
> A
EOF seems wrong to me (not as bad as EFAULT, but still wrong).
-hpa
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] /dev/mem: Fix wrong error on accessing beyond valid memory addresses.
2011-11-14 21:59 ` H. Peter Anvin
@ 2011-11-14 22:11 ` Andrei Warkentin
2011-11-14 22:12 ` H. Peter Anvin
0 siblings, 1 reply; 6+ messages in thread
From: Andrei Warkentin @ 2011-11-14 22:11 UTC (permalink / raw)
To: H. Peter Anvin; +Cc: linux-kernel
Hi Peter,
----- Original Message -----
> From: "H. Peter Anvin" <hpa@zytor.com>
> To: "Andrei Warkentin" <awarkentin@vmware.com>
> Cc: linux-kernel@vger.kernel.org
> Sent: Monday, November 14, 2011 4:59:58 PM
> Subject: Re: [PATCH] /dev/mem: Fix wrong error on accessing beyond valid memory addresses.
>
> On 11/14/2011 01:21 PM, Andrei Warkentin wrote:
> >
> > I know you're very busy, but I was wondering if you could
> > give your two cents on this change. This change grew from
> > our discussion on LKML last week.
> >
> > Thanks ahead,
> > A
>
> EOF seems wrong to me (not as bad as EFAULT, but still wrong).
Although that was what we had discussed, I do not return -EOF in the patch. As you
mention - that would be wrong. I played around with other devices (block, actually),
and if you attempt to read(2) beyond the end, read(2) simply returns 0, as in
zero bytes read out. Of course, lseek(2) beyond the end should return -EINVAL, as
well, so that is what that patch I CCd you on accomplishes.
A
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] /dev/mem: Fix wrong error on accessing beyond valid memory addresses.
2011-11-14 22:11 ` Andrei Warkentin
@ 2011-11-14 22:12 ` H. Peter Anvin
2011-11-14 22:33 ` Andrei Warkentin
0 siblings, 1 reply; 6+ messages in thread
From: H. Peter Anvin @ 2011-11-14 22:12 UTC (permalink / raw)
To: Andrei Warkentin; +Cc: linux-kernel
On 11/14/2011 02:11 PM, Andrei Warkentin wrote:
>>
>> EOF seems wrong to me (not as bad as EFAULT, but still wrong).
>
> Although that was what we had discussed, I do not return -EOF in the patch. As you
> mention - that would be wrong. I played around with other devices (block, actually),
> and if you attempt to read(2) beyond the end, read(2) simply returns 0, as in
> zero bytes read out. Of course, lseek(2) beyond the end should return -EINVAL, as
> well, so that is what that patch I CCd you on accomplishes.
>
0 is EOF.
This is not a block device -- comparing to block devices is pointless.
There is no "end" to /dev/mem, so this is a totally meaningless comparison.
-hpa
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] /dev/mem: Fix wrong error on accessing beyond valid memory addresses.
2011-11-14 22:12 ` H. Peter Anvin
@ 2011-11-14 22:33 ` Andrei Warkentin
0 siblings, 0 replies; 6+ messages in thread
From: Andrei Warkentin @ 2011-11-14 22:33 UTC (permalink / raw)
To: H. Peter Anvin; +Cc: linux-kernel
Hi,
----- Original Message -----
> From: "H. Peter Anvin" <hpa@zytor.com>
> To: "Andrei Warkentin" <awarkentin@vmware.com>
> Cc: linux-kernel@vger.kernel.org
> Sent: Monday, November 14, 2011 5:12:35 PM
> Subject: Re: [PATCH] /dev/mem: Fix wrong error on accessing beyond valid memory addresses.
>
> On 11/14/2011 02:11 PM, Andrei Warkentin wrote:
> >>
> >> EOF seems wrong to me (not as bad as EFAULT, but still wrong).
> >
> > Although that was what we had discussed, I do not return -EOF in
> > the patch. As you
> > mention - that would be wrong. I played around with other devices
> > (block, actually),
> > and if you attempt to read(2) beyond the end, read(2) simply
> > returns 0, as in
> > zero bytes read out. Of course, lseek(2) beyond the end should
> > return -EINVAL, as
> > well, so that is what that patch I CCd you on accomplishes.
> >
>
> 0 is EOF.
>
Ah, whoops, silly me.
> This is not a block device -- comparing to block devices is
> pointless.
> There is no "end" to /dev/mem, so this is a totally meaningless
> comparison.
Oh ok, I looked at other arches, like IA64, and now see why
using valid_phys_addr_range as an "end" test is meaningless.
Thanks,
A
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-11-14 22:33 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-11-09 23:31 [PATCH] /dev/mem: Fix wrong error on accessing beyond valid memory addresses Andrei Warkentin
2011-11-14 21:21 ` Andrei Warkentin
2011-11-14 21:59 ` H. Peter Anvin
2011-11-14 22:11 ` Andrei Warkentin
2011-11-14 22:12 ` H. Peter Anvin
2011-11-14 22:33 ` Andrei Warkentin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox