Bug 11057 - vbetest fails on 2.6.26-rc9
Summary: vbetest fails on 2.6.26-rc9
Status: CLOSED CODE_FIX
Alias: None
Product: Memory Management
Classification: Unclassified
Component: Other (show other bugs)
Hardware: All Linux
: P1 normal
Assignee: Thomas Gleixner
URL:
Keywords:
Depends on:
Blocks: 10492
  Show dependency tree
 
Reported: 2008-07-08 19:46 UTC by PaX Team
Modified: 2008-07-13 12:50 UTC (History)
3 users (show)

See Also:
Kernel Version:
Subsystem:
Regression: Yes
Bisected commit-id:


Attachments
failing .config (60.75 KB, application/octet-stream)
2008-07-09 11:35 UTC, PaX Team
Details
proposed patch to fix this bug (1.76 KB, patch)
2008-07-09 17:44 UTC, Venkatesh Pallipadi
Details | Diff
Updated patch (1.81 KB, patch)
2008-07-10 09:40 UTC, Venkatesh Pallipadi
Details | Diff

Description PaX Team 2008-07-08 19:46:17 UTC
Latest working kernel version: 2.6.25.x
Earliest failing kernel version: 2.6.26-rcX
Distribution:
Hardware Environment: i386
Software Environment:
Problem Description:
vbetest fails due to changes to read_mem/xlate_dev_mem_ptr probably. strace excerpt:

open("/dev/mem", O_RDONLY)              = 3
read(3, 0, 1282)                        = -1 EFAULT (Bad address)

xlate_dev_mem_ptr used to be a simple __va() on i386, now it calls page_is_ram which will report the first page as not ram, which in turn ends up ioremap'ing that page which also fails. of course a simple dd if=/dev/mem also fails the same way.

Steps to reproduce:
vbetest or 'dd if=/dev/mem'
Comment 1 Anonymous Emailer 2008-07-08 21:00:59 UTC
Reply-To: akpm@linux-foundation.org

On Tue,  8 Jul 2008 19:46:17 -0700 (PDT) bugme-daemon@bugzilla.kernel.org wrote:

> http://bugzilla.kernel.org/show_bug.cgi?id=11057
> 
>            Summary: vbetest fails on 2.6.26-rc9
>            Product: Memory Management
>            Version: 2.5
>           Platform: All
>         OS/Version: Linux
>               Tree: Mainline
>             Status: NEW
>           Severity: normal
>           Priority: P1
>          Component: Other
>         AssignedTo: akpm@osdl.org
>         ReportedBy: pageexec@freemail.hu
> 
> 
> Latest working kernel version: 2.6.25.x
> Earliest failing kernel version: 2.6.26-rcX
> Distribution:
> Hardware Environment: i386
> Software Environment:
> Problem Description:
> vbetest fails due to changes to read_mem/xlate_dev_mem_ptr probably. strace
> excerpt:
> 
> open("/dev/mem", O_RDONLY)              = 3
> read(3, 0, 1282)                        = -1 EFAULT (Bad address)
> 
> xlate_dev_mem_ptr used to be a simple __va() on i386, now it calls
> page_is_ram
> which will report the first page as not ram, which in turn ends up
> ioremap'ing
> that page which also fails. of course a simple dd if=/dev/mem also fails the
> same way.
> 
> Steps to reproduce:
> vbetest or 'dd if=/dev/mem'
> 

Thanks.  afaik this wasn't known about before?
Comment 2 Thomas Gleixner 2008-07-08 22:46:46 UTC
Do you have CONFIG_NONPROMISC_DEVMEM enabled ?
Comment 3 PaX Team 2008-07-09 01:51:05 UTC
no, it isn't:

# CONFIG_NONPROMISC_DEVMEM is not set

note that failing the range_is_allowed() check would produce an EPERM in read_mem and EINVAL in mmap_mem (that's an inconsistency worth fixing maybe btw), not EFAULT.
Comment 4 Thomas Gleixner 2008-07-09 05:02:16 UTC
On Wed, 9 Jul 2008, bugme-daemon@bugzilla.kernel.org wrote:
> no, it isn't:
> 
> # CONFIG_NONPROMISC_DEVMEM is not set
> 
> note that failing the range_is_allowed() check would produce an EPERM in
> read_mem and EINVAL in mmap_mem (that's an inconsistency worth fixing maybe
> btw), not EFAULT.

Oops, right. But looking at your strace outout again:

read(3, 0, 1282)                        = -1 EFAULT (Bad address)

--------^

definitely causes an EFAULT :)
Comment 5 PaX Team 2008-07-09 05:21:06 UTC
no, it won't, here's more context :)

open("/dev/zero", O_RDONLY)             = 3
mmap2(NULL, 1282, PROT_READ|PROT_WRITE|PROT_EXEC, MAP_PRIVATE|MAP_FIXED, 3, 0) = 0
close(3)                                = 0
open("/dev/mem", O_RDONLY)              = 3
read(3, 0, 1282)                        = -1 EFAULT (Bad address)

so virtual address 0 is a valid mapping, the EFAULT is a regression (imho). and if you check the 'dd if=/dev/mem' case, it'll be even more obvious:

brk(0x807c000)                          = 0x807c000
[...]
open("/dev/mem", O_RDONLY|O_LARGEFILE)  = 3
dup2(3, 0)                              = 0
[...]
_llseek(0, 0, [0], SEEK_CUR)            = 0
[...]
read(0, 0x805c000, 512)                 = -1 EFAULT (Bad address)
Comment 6 Rafael J. Wysocki 2008-07-09 07:27:58 UTC
This entry is being used for tracking a regression from 2.6.25.  Please don't
close it until the problem is fixed in the mainline.
Comment 7 Venkatesh Pallipadi 2008-07-09 10:46:32 UTC
Can you share the .config that shows up the problem and also the dmesg from the system.

Tried reproducing on few local systems, but both read of /dev/mem and dd goes through fine using the ioremap() in xlate().
Comment 8 Venkatesh Pallipadi 2008-07-09 10:59:56 UTC
Clarifying on what I tested on my end

#dd if=/dev/mem of=/tmp/1 bs=4096 count=1

works fine without any error.
Comment 9 PaX Team 2008-07-09 11:35:18 UTC
Created attachment 16773 [details]
failing .config

here's a .config from a box where i ran into this problem by chance, it definitely fails both vbetest and dd. let me know if dmesg is still needed, it's somewhat too verbose to disclose for my taste ;).
Comment 10 Venkatesh Pallipadi 2008-07-09 14:54:47 UTC
dmesg is required to see that there is nothing wierd with e820 table on this box.

Other potential issue is that this address (0-4095 physical) is mapped by some kernel or driver as WB and we are having a conflict here when vbetest tries to map it UC_MINUS, causing ioremap() to fail.
Comment 11 Venkatesh Pallipadi 2008-07-09 17:44:47 UTC
Created attachment 16778 [details]
proposed patch to fix this bug

Can you please test whether this patch resolves the problem?
Comment 12 Ingo Molnar 2008-07-10 01:31:35 UTC
applied the fix to tip/x86/urgent.
Comment 13 PaX Team 2008-07-10 05:34:19 UTC
i confirm that the patch fixes both vbetest and dd, thanks! small note on whitespace: some of the comment lines are indented with all spaces, not tab+space. also 'ret' could be declared as 'void __iomem *' to avoid the casts. 
Comment 14 Venkatesh Pallipadi 2008-07-10 09:40:32 UTC
Created attachment 16792 [details]
Updated patch

Yes. I noticed the whitespace messup after attaching the patch yday. Here is the an updated patch with whitespace and void __iomem * cleanups.
Comment 15 Rafael J. Wysocki 2008-07-13 11:42:33 UTC
Handled-By : Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
Patch : http://bugzilla.kernel.org/attachment.cgi?id=16792&action=view

Note You need to log in before you can comment on or make changes to this bug.