All of lore.kernel.org
 help / color / mirror / Atom feed
From: "H. Peter Anvin" <hpa@zytor.com>
To: Frantisek Hrbata <fhrbata@redhat.com>
Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	tglx@linutronix.de, mingo@redhat.com, x86@kernel.org,
	oleg@redhat.com, kamaleshb@in.ibm.com, hechjie@cn.ibm.com
Subject: Re: [PATCH] x86: add phys addr validity check for /dev/mem mmap
Date: Tue, 02 Apr 2013 13:29:12 -0700	[thread overview]
Message-ID: <515B3F98.5020101@zytor.com> (raw)
In-Reply-To: <20130402191012.GC3314@dhcp-26-164.brq.redhat.com>

On 04/02/2013 12:10 PM, Frantisek Hrbata wrote:
> 
> Hi, this is exactly what the patch is doing imho. Note that the
> valid_phys_addr_range(), which is using the high_memory, is the same as the
> default one in drivers/char/mem.c(#ifndef ARCH_HAS_VALID_PHYS_ADDR_RANGE). I
> just added x86 specific check for valid_mmap_phys_addr_range and moved both
> functions to arch/x86/mm/mmap.c, rather then modifying the default generic ones.
> This is how other archs(arm) are doing it.
> 
> Also valid_phys_addr_range is used just in read|write_mem and
> valid_mmap_phys_addr_range is checked in mmap_mem and it calls phys_addr_valid
> 
> static inline int phys_addr_valid(resource_size_t addr)
> {
> #ifdef CONFIG_PHYS_ADDR_T_64BIT
> 	return !(addr >> boot_cpu_data.x86_phys_bits);
> #else
>         return 1;
> #endif
> }                          
> 
> I for sure could overlooked something, but this seems right to me.
> 

OK, this is really confusing ... which isn't a *huge* surprise (the
entire /dev/mem code has some gigantic bugs in it.)

I think I need to do more of an in-depth review.  The other question is
why we don't call phys_addr_valid() everywhere.

	-hpa


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

WARNING: multiple messages have this Message-ID (diff)
From: "H. Peter Anvin" <hpa@zytor.com>
To: Frantisek Hrbata <fhrbata@redhat.com>
Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	tglx@linutronix.de, mingo@redhat.com, x86@kernel.org,
	oleg@redhat.com, kamaleshb@in.ibm.com, hechjie@cn.ibm.com
Subject: Re: [PATCH] x86: add phys addr validity check for /dev/mem mmap
Date: Tue, 02 Apr 2013 13:29:12 -0700	[thread overview]
Message-ID: <515B3F98.5020101@zytor.com> (raw)
In-Reply-To: <20130402191012.GC3314@dhcp-26-164.brq.redhat.com>

On 04/02/2013 12:10 PM, Frantisek Hrbata wrote:
> 
> Hi, this is exactly what the patch is doing imho. Note that the
> valid_phys_addr_range(), which is using the high_memory, is the same as the
> default one in drivers/char/mem.c(#ifndef ARCH_HAS_VALID_PHYS_ADDR_RANGE). I
> just added x86 specific check for valid_mmap_phys_addr_range and moved both
> functions to arch/x86/mm/mmap.c, rather then modifying the default generic ones.
> This is how other archs(arm) are doing it.
> 
> Also valid_phys_addr_range is used just in read|write_mem and
> valid_mmap_phys_addr_range is checked in mmap_mem and it calls phys_addr_valid
> 
> static inline int phys_addr_valid(resource_size_t addr)
> {
> #ifdef CONFIG_PHYS_ADDR_T_64BIT
> 	return !(addr >> boot_cpu_data.x86_phys_bits);
> #else
>         return 1;
> #endif
> }                          
> 
> I for sure could overlooked something, but this seems right to me.
> 

OK, this is really confusing ... which isn't a *huge* surprise (the
entire /dev/mem code has some gigantic bugs in it.)

I think I need to do more of an in-depth review.  The other question is
why we don't call phys_addr_valid() everywhere.

	-hpa



  reply	other threads:[~2013-04-02 20:29 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-02 12:28 [PATCH] x86: add phys addr validity check for /dev/mem mmap Frantisek Hrbata
2013-04-02 12:28 ` Frantisek Hrbata
2013-04-02 18:02 ` Oleg Nesterov
2013-04-02 18:02   ` Oleg Nesterov
2013-04-02 18:48 ` H. Peter Anvin
2013-04-02 18:48   ` H. Peter Anvin
2013-04-02 19:10   ` Frantisek Hrbata
2013-04-02 19:10     ` Frantisek Hrbata
2013-04-02 20:29     ` H. Peter Anvin [this message]
2013-04-02 20:29       ` H. Peter Anvin
2013-04-02 20:52       ` Frantisek Hrbata
2013-04-02 20:52         ` Frantisek Hrbata
2013-04-24 11:36       ` Frantisek Hrbata
2013-04-24 11:36         ` Frantisek Hrbata
2013-04-04  1:11   ` Simon Jeons
2013-04-04  1:11     ` Simon Jeons
2013-04-04  1:13     ` H. Peter Anvin
2013-04-04  1:13       ` H. Peter Anvin
2013-04-04  1:17       ` Simon Jeons
2013-04-04  1:17         ` Simon Jeons
2013-04-04  1:32         ` H. Peter Anvin
2013-04-04  1:32           ` H. Peter Anvin
2013-04-04  1:53           ` Simon Jeons
2013-04-04  1:53             ` Simon Jeons
2013-04-04  2:14             ` H. Peter Anvin
2013-04-04  2:14               ` H. Peter Anvin
2013-04-04  2:17               ` Simon Jeons
2013-04-04  2:17                 ` Simon Jeons
2013-04-04  5:20           ` Simon Jeons
2013-04-04  5:20             ` Simon Jeons
2013-04-11  2:40       ` Simon Jeons
2013-04-11  2:40         ` Simon Jeons
2013-04-11  2:48         ` H. Peter Anvin
2013-04-11  2:48           ` H. Peter Anvin
2013-04-11  2:58           ` Simon Jeons
2013-04-11  2:58             ` Simon Jeons
2013-04-03  2:46 ` Cheng Jie He
2013-04-26  5:21 ` Will Huck
2013-04-26  5:21   ` Will Huck
2013-04-26 15:35   ` Frantisek Hrbata
2013-04-26 15:35     ` Frantisek Hrbata
2013-04-27  7:00     ` Will Huck
2013-04-27  7:00       ` Will Huck
2013-04-27 19:13       ` Frantisek Hrbata
2013-04-27 19:13         ` Frantisek Hrbata
2013-04-28  3:17         ` Will Huck
2013-04-28  3:17           ` Will Huck
2013-04-28  4:00           ` H. Peter Anvin
2013-04-28  4:00             ` H. Peter Anvin
2013-04-28  8:03             ` Will Huck
2013-04-28  8:03               ` Will Huck
2013-05-01 18:19           ` Dave Hansen
2013-05-01 18:19             ` Dave Hansen
2013-05-01 19:04             ` Frantisek Hrbata
2013-05-01 19:04               ` Frantisek Hrbata

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=515B3F98.5020101@zytor.com \
    --to=hpa@zytor.com \
    --cc=fhrbata@redhat.com \
    --cc=hechjie@cn.ibm.com \
    --cc=kamaleshb@in.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mingo@redhat.com \
    --cc=oleg@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.