Re: Patch [1/1] minor bugfix in 2.6.26/arch/x86/mm/pat.c - caused problems in mmap() of /dev/mem character file

["V.Radhakrishnan" <rk@atr-labs.com>]

Hi,

Thanks !

I was teaching a class of 17 students from HP, basics of kernel
debugging. I had already burnt CDs for them with 2.6.25, and I thought I
would try out the latest 2.6.26 ( announced just a day before the start
of the training program ) last week.

I wanted to cover basic concepts of memory management, and used
the /dev/mem purely as a means to show them what the physical memory
contains, how you can use this mechanism to view the contents of RAM
after a DMA to verify data patterns etc.

It so happened that all my 17 students had working code with 2.6.25 and
I, the instructor, had the kernel 2.6.26 crash in front of them !

So I started tracing the call and found the 'bug' and 'fixed it'.

I agree with you 100% that my patch does not solve the problem, but is a
quick fix approach to the next release. The PAT related access issue is
valid. However, I humbly submit that memory access is a policy issue and
perhaps should not be tinkered with thru code. 

If you provide the root, access to /dev/mem, I don't think it should be
a problem, since in any case, all others are denied access. Also,
theoretically, if you access RAM for Read Only, why should there be a
triple fault ? Again, if you access RAM, modify content, and mark those
pages as dirty ( as described in LDD ), isnt that supposed to take care
of cache issue ? Again, I re-iterate, I am NOT suggesting writing
directly to RAM !, But what about Read Access which which is what
several simple debuggers do ? The crash utility from RedHat also reads
memory (both user as well as kernel virtual addresses as input ). Is
reading RAM directly using a physical address deemed 'bad' ?

Thanks, anyway, and really happy that you have taken the time to answer
these issues.

regards

V. Radhakrishnan (alias 'RK')



On Sat, 2008-07-19 at 15:05 -0700, Arjan van de Ven wrote:
> On Fri, 18 Jul 2008 07:34:17 +0530
> "V.Radhakrishnan" <rk@atr-labs.com> wrote:
> > > But this duplication is ugly and confusing - we should have just a 
> > > single check function, defined once and unconditionally, and
> > > utilized by both places (with the devmem check being optional).
> > > 
> > > Venki, Arjan, agreed?
> > 
> > In fact, I spent some time thinking that the bug was actually at the
> > higher level API instead of in the arch specific layer, and I was
> > amazed that in spite of the config level option being turned off, it
> > was NOT being reflected in the code !!
> 
> Hi,
> 
> it's great to see new people contribute to the kernel, however your
> patch is not correct. In the PAT case, we really cannot allow /dev/mem
> mmap access of kernel-used memory. It would create an incorrect cache
> alias... which can have really bad effects, including tripple faulting
> the cpu (which is an instant reboot) or data corruption, depending on
> which model/vendor CPU you have.
> 
> What your patch does is to remove the code that prevents this situation
> from happening... and that makes it not a very good idea.
> 
> Now, /dev/mem mmap access of address space that is not used by the
> kernel is still allowed by the code (for example the PCI mmio region)...
> 
> Can you describe what you were trying to achieve by mmaping
> of /dev/mem ? It sounds like you have a bug, since it's certainly
> curious that you would want to deal with kernel memory this way.
> (for example until recently, you wouldn't be able to do this at all)
> 
> Greetings,
>    Arjan van de Ven
> 
>