public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Arjan van de Ven <arjan@infradead.org>
To: "V.Radhakrishnan" <rk@atr-labs.com>
Cc: Ingo Molnar <mingo@elte.hu>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Linux Kernel Mailing List <Linux-kernel@vger.kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Suresh Siddha <suresh.b.siddha@intel.com>,
	"H. Peter Anvin" <hpa@zytor.com>,
	Venki Pallipadi <venkatesh.pallipadi@intel.com>
Subject: Re: Patch [1/1] minor bugfix in 2.6.26/arch/x86/mm/pat.c - caused problems in mmap() of /dev/mem character file
Date: Sat, 19 Jul 2008 15:05:46 -0700	[thread overview]
Message-ID: <20080719150546.15a0e773@infradead.org> (raw)
In-Reply-To: <1216346657.2170.11.camel@atlas>

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


-- 
If you want to reach me at my work email, use arjan@linux.intel.com
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

  reply	other threads:[~2008-07-19 22:07 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-07-17 17:25 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
2008-07-17 17:27 ` Linus Torvalds
2008-07-17 22:39   ` Ingo Molnar
2008-07-18  2:04     ` V.Radhakrishnan
2008-07-19 22:05       ` Arjan van de Ven [this message]
2008-07-20 15:50         ` V.Radhakrishnan
2008-07-20 15:56           ` Arjan van de Ven
2008-07-19 22:47   ` Arjan van de Ven
2008-07-20  6:37     ` Ingo Molnar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080719150546.15a0e773@infradead.org \
    --to=arjan@infradead.org \
    --cc=Linux-kernel@vger.kernel.org \
    --cc=hpa@zytor.com \
    --cc=mingo@elte.hu \
    --cc=rk@atr-labs.com \
    --cc=suresh.b.siddha@intel.com \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=venkatesh.pallipadi@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox