[PATCH] hpet, allow user controlled mmap for user processes

[PATCH] hpet, allow user controlled mmap for user processes

[Prarit Bhargava]

The CONFIG_HPET_MMAP Kconfig option exposes the memory map of the HPET
registers to userspace.  The Kconfig help points out that in some cases this
can be a security risk as some systems may erroneously configure the map such
that additional data is exposed to userspace.

This is a problem for distributions -- some users want the MMAP functionality
can verify that their systems are secure, but it comes with a significant
security risk for those who do not want the functionality.  In an effort
to mitigate this risk, and due to the low number of users of the MMAP
functionality I've introduced a kernel parameter, hpet_mmap_enable, that
is required in order to actually have the HPET MMAP exposed.

Signed-off-by: Prarit Bhargava <prarit@redhat.com>
Cc: Clemens Ladisch <clemens@ladisch.de>
---
 Documentation/kernel-parameters.txt |    3 +++
 drivers/char/hpet.c                 |   20 ++++++++++++++++++--
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index e567af3..dbf0d81 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -962,6 +962,9 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 				VIA, nVidia)
 			verbose: show contents of HPET registers during setup
 
+	hpet_mmap_enable [X86, HPET_MMAP] option to expose HPET MMAP to
+			 userspace.  By default this is disabled.
+
 	hugepages=	[HW,X86-32,IA-64] HugeTLB pages to allocate at boot.
 	hugepagesz=	[HW,IA-64,PPC,X86-64] The size of the HugeTLB pages.
 			On x86-64 and powerpc, this option can be specified
diff --git a/drivers/char/hpet.c b/drivers/char/hpet.c
index e3f9a99..de770ab 100644
--- a/drivers/char/hpet.c
+++ b/drivers/char/hpet.c
@@ -367,12 +367,25 @@ static unsigned int hpet_poll(struct file *file, poll_table * wait)
 	return 0;
 }
 
+#ifdef	CONFIG_HPET_MMAP
+static int hpet_mmap_enabled;
+
+static __init int hpet_mmap_enable(char *str)
+{
+	pr_info(KERN_INFO "HPET MMAP enabled\n");
+	hpet_mmap_enabled = 1;
+	return 1;
+}
+__setup("hpet_mmap_enable", hpet_mmap_enable);
+
 static int hpet_mmap(struct file *file, struct vm_area_struct *vma)
 {
-#ifdef	CONFIG_HPET_MMAP
 	struct hpet_dev *devp;
 	unsigned long addr;
 
+	if (!hpet_mmap_enabled)
+		return -EACCES;
+
 	if (((vma->vm_end - vma->vm_start) != PAGE_SIZE) || vma->vm_pgoff)
 		return -EINVAL;
 
@@ -393,10 +406,13 @@ static int hpet_mmap(struct file *file, struct vm_area_struct *vma)
 	}
 
 	return 0;
+}
 #else
+static int hpet_mmap(struct file *file, struct vm_area_struct *vma)
+{
 	return -ENOSYS;
-#endif
 }
+#endif
 
 static int hpet_fasync(int fd, struct file *file, int on)
 {
-- 
1.7.9.3

Re: [PATCH] hpet, allow user controlled mmap for user processes

[Clemens Ladisch]

Prarit Bhargava wrote:
> The CONFIG_HPET_MMAP Kconfig option exposes the memory map of the HPET
> registers to userspace.  The Kconfig help points out that in some cases this
> can be a security risk as some systems may erroneously configure the map such
> that additional data is exposed to userspace.

I'm not aware of any such system (but cannot rule out the possibility).

> In an effort to mitigate this risk, and due to the low number of users
> of the MMAP functionality I've introduced a kernel parameter,
> hpet_mmap_enable, that is required in order to actually have the HPET
> MMAP exposed.

This introduces a regression for all users.  At least make the default
state (allowed/forbidden) configurable.

Also, this patch makes the Kconfig help text a lie.


Regards,
Clemens

Re: [PATCH] hpet, allow user controlled mmap for user processes

[Prarit Bhargava]

The CONFIG_HPET_MMAP Kconfig option exposes the memory map of the HPET
registers to userspace.  The Kconfig help points out that in some cases this
can be a security risk as some systems may erroneously configure the map such
that additional data is exposed to userspace.

This is a problem for distributions -- some users want the MMAP functionality
but it comes with a significant security risk.  In an effort to mitigate this
risk, and due to the low number of users of the MMAP functionality, I've
introduced a kernel parameter, hpet_mmap_enable, that is required in order
to actually have the HPET MMAP exposed.

[v2]: Clemens suggested modifying the Kconfig help text and making the
default setting configurable.

Signed-off-by: Prarit Bhargava <prarit@redhat.com>
Cc: Clemens Ladisch <clemens@ladisch.de>
---
 Documentation/kernel-parameters.txt |    3 +++
 drivers/char/Kconfig                |   10 ++++++++--
 drivers/char/hpet.c                 |   21 +++++++++++++++++++--
 3 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index e567af3..dbf0d81 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -962,6 +962,9 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 				VIA, nVidia)
 			verbose: show contents of HPET registers during setup
 
+	hpet_mmap_enable [X86, HPET_MMAP] option to expose HPET MMAP to
+			 userspace.  By default this is disabled.
+
 	hugepages=	[HW,X86-32,IA-64] HugeTLB pages to allocate at boot.
 	hugepagesz=	[HW,IA-64,PPC,X86-64] The size of the HugeTLB pages.
 			On x86-64 and powerpc, this option can be specified
diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig
index 3bb6fa3..7b830aa 100644
--- a/drivers/char/Kconfig
+++ b/drivers/char/Kconfig
@@ -534,10 +534,16 @@ config HPET_MMAP
 	  If you say Y here, user applications will be able to mmap
 	  the HPET registers.
 
+config HPET_MMAP_DEFAULT
+	int "Enable HPET MMAP access by default"
+	depends on HPET_MMAP
+	range 0 1
+	default 0
+	help
 	  In some hardware implementations, the page containing HPET
 	  registers may also contain other things that shouldn't be
-	  exposed to the user.  If this applies to your hardware,
-	  say N here.
+	  exposed to the user. This option selects the default user access
+	  to the HPET registers for applications that require it.  (0=off, 1=on)
 
 config HANGCHECK_TIMER
 	tristate "Hangcheck timer"
diff --git a/drivers/char/hpet.c b/drivers/char/hpet.c
index e3f9a99..0f32e30 100644
--- a/drivers/char/hpet.c
+++ b/drivers/char/hpet.c
@@ -367,12 +367,26 @@ static unsigned int hpet_poll(struct file *file, poll_table * wait)
 	return 0;
 }
 
+#ifdef CONFIG_HPET_MMAP
+static int hpet_mmap_enabled = CONFIG_HPET_MMAP_DEFAULT;
+
+static __init int hpet_mmap_enable(char *str)
+{
+	get_option(&str, &hpet_mmap_enabled);
+	pr_info(KERN_INFO "HPET MMAP %s\n",
+		hpet_mmap_enabled ? "disabled" : "enabled");
+	return 1;
+}
+__setup("hpet_mmap", hpet_mmap_enable);
+
 static int hpet_mmap(struct file *file, struct vm_area_struct *vma)
 {
-#ifdef	CONFIG_HPET_MMAP
 	struct hpet_dev *devp;
 	unsigned long addr;
 
+	if (!hpet_mmap_enabled)
+		return -EACCES;
+
 	if (((vma->vm_end - vma->vm_start) != PAGE_SIZE) || vma->vm_pgoff)
 		return -EINVAL;
 
@@ -393,10 +407,13 @@ static int hpet_mmap(struct file *file, struct vm_area_struct *vma)
 	}
 
 	return 0;
+}
 #else
+static int hpet_mmap(struct file *file, struct vm_area_struct *vma)
+{
 	return -ENOSYS;
-#endif
 }
+#endif
 
 static int hpet_fasync(int fd, struct file *file, int on)
 {
-- 
1.7.9.3

Re: [PATCH] hpet, allow user controlled mmap for user processes

[Clemens Ladisch]

Prarit Bhargava wrote:
> The CONFIG_HPET_MMAP Kconfig option exposes the memory map of the HPET
> registers to userspace.  The Kconfig help points out that in some cases this
> can be a security risk as some systems may erroneously configure the map such
> that additional data is exposed to userspace.
>
> This is a problem for distributions -- some users want the MMAP functionality
> but it comes with a significant security risk.  In an effort to mitigate this
> risk, and due to the low number of users of the MMAP functionality, I've
> introduced a kernel parameter, hpet_mmap_enable, that is required in order
> to actually have the HPET MMAP exposed.
>
> [v2]: Clemens suggested modifying the Kconfig help text and making the
> default setting configurable.
>
> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
> Cc: Clemens Ladisch <clemens@ladisch.de>

> +++ b/Documentation/kernel-parameters.txt
> +	hpet_mmap_enable [X86, HPET_MMAP] option to expose HPET MMAP to
> +			 userspace.  By default this is disabled.

This now takes a value.

> +	int "Enable HPET MMAP access by default"
> +	range 0 1

Shouldn't this be bool?

> +	default 0

This breaks backwards compatibility.


Regards,
Clemens

Re: [PATCH] hpet, allow user controlled mmap for user processes

[Prarit Bhargava]

On 03/19/2013 03:43 AM, Clemens Ladisch wrote:
> Prarit Bhargava wrote:
>> The CONFIG_HPET_MMAP Kconfig option exposes the memory map of the HPET
>> registers to userspace.  The Kconfig help points out that in some cases this
>> can be a security risk as some systems may erroneously configure the map such
>> that additional data is exposed to userspace.
>>
>> This is a problem for distributions -- some users want the MMAP functionality
>> but it comes with a significant security risk.  In an effort to mitigate this
>> risk, and due to the low number of users of the MMAP functionality, I've
>> introduced a kernel parameter, hpet_mmap_enable, that is required in order
>> to actually have the HPET MMAP exposed.
>>
>> [v2]: Clemens suggested modifying the Kconfig help text and making the
>> default setting configurable.
>>
>> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
>> Cc: Clemens Ladisch <clemens@ladisch.de>
> 
>> +++ b/Documentation/kernel-parameters.txt
>> +	hpet_mmap_enable [X86, HPET_MMAP] option to expose HPET MMAP to
>> +			 userspace.  By default this is disabled.
> 
> This now takes a value.
> 
>> +	int "Enable HPET MMAP access by default"
>> +	range 0 1
> 
> Shouldn't this be bool?

I'll fix those in v3.

> 
>> +	default 0
> 
> This breaks backwards compatibility.

Does backwards compatibility matter for something like?  I have no problem
setting it to 1 but I'm more curious from a general kernel point of view.

I'll change this in v3 as well.

P.


P.

> 
> 
> Regards,
> Clemens

Re: [PATCH] hpet, allow user controlled mmap for user processes

[Clemens Ladisch]

Prarit Bhargava wrote:
> On 03/19/2013 03:43 AM, Clemens Ladisch wrote:
>> Prarit Bhargava wrote:
>>> +	int "Enable HPET MMAP access by default"
>>> +	default 0
>>
>> This breaks backwards compatibility.
>
> Does backwards compatibility matter for something like?  I have no problem
> setting it to 1 but I'm more curious from a general kernel point of view.

When somebody updates from some old kernel where HPET mmap was
explicitly enabled, this default value will now disable it.  The
default behaviour should always be to be compatible with older
kernels.


Regards,
Clemens

[PATCH] hpet, allow user controlled mmap for user processes

[Prarit Bhargava]

The CONFIG_HPET_MMAP Kconfig option exposes the memory map of the HPET
registers to userspace.  The Kconfig help points out that in some cases this
can be a security risk as some systems may erroneously configure the map such
that additional data is exposed to userspace.

This is a problem for distributions -- some users want the MMAP functionality
but it comes with a significant security risk.  In an effort to mitigate this
risk, and due to the low number of users of the MMAP functionality, I've
introduced a kernel parameter, hpet_mmap_enable, that is required in order
to actually have the HPET MMAP exposed.

[v2]: Clemens suggested modifying the Kconfig help text and making the
default setting configurable.
[v3]: Fixed up Documentation and Kconfig entries, default now "Y"
[v4]: After testing, found that I need to modify CONFIG_HPET_MMAP_DEFAULT usage

Signed-off-by: Prarit Bhargava <prarit@redhat.com>
Cc: Clemens Ladisch <clemens@ladisch.de>
---
 Documentation/kernel-parameters.txt |    4 ++++
 drivers/char/Kconfig                |    9 +++++++--
 drivers/char/hpet.c                 |   25 +++++++++++++++++++++++--
 3 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index e567af3..1444491 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -962,6 +962,10 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 				VIA, nVidia)
 			verbose: show contents of HPET registers during setup
 
+	hpet_mmap=	[X86, HPET_MMAP] option to expose HPET MMAP to
+			userspace.  By default this is disabled. Values are
+			0(disabled) or 1(enabled).
+
 	hugepages=	[HW,X86-32,IA-64] HugeTLB pages to allocate at boot.
 	hugepagesz=	[HW,IA-64,PPC,X86-64] The size of the HugeTLB pages.
 			On x86-64 and powerpc, this option can be specified
diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig
index 3bb6fa3..51b62a1 100644
--- a/drivers/char/Kconfig
+++ b/drivers/char/Kconfig
@@ -534,10 +534,15 @@ config HPET_MMAP
 	  If you say Y here, user applications will be able to mmap
 	  the HPET registers.
 
+config HPET_MMAP_DEFAULT
+	bool "Enable HPET MMAP access by default"
+	default y
+	depends on HPET_MMAP
+	help
 	  In some hardware implementations, the page containing HPET
 	  registers may also contain other things that shouldn't be
-	  exposed to the user.  If this applies to your hardware,
-	  say N here.
+	  exposed to the user. This option selects the default user access
+	  to the HPET registers for applications that require it.
 
 config HANGCHECK_TIMER
 	tristate "Hangcheck timer"
diff --git a/drivers/char/hpet.c b/drivers/char/hpet.c
index e3f9a99..b3ba043 100644
--- a/drivers/char/hpet.c
+++ b/drivers/char/hpet.c
@@ -367,12 +367,30 @@ static unsigned int hpet_poll(struct file *file, poll_table * wait)
 	return 0;
 }
 
+#ifdef CONFIG_HPET_MMAP
+#ifdef CONFIG_HPET_MMAP_DEFAULT
+static int hpet_mmap_enabled = 1;
+#else
+static int hpet_mmap_enabled = 0;
+#endif
+
+static __init int hpet_mmap_enable(char *str)
+{
+	get_option(&str, &hpet_mmap_enabled);
+	pr_info(KERN_INFO "HPET MMAP %s\n",
+		hpet_mmap_enabled ? "disabled" : "enabled");
+	return 1;
+}
+__setup("hpet_mmap", hpet_mmap_enable);
+
 static int hpet_mmap(struct file *file, struct vm_area_struct *vma)
 {
-#ifdef	CONFIG_HPET_MMAP
 	struct hpet_dev *devp;
 	unsigned long addr;
 
+	if (!hpet_mmap_enabled)
+		return -EACCES;
+
 	if (((vma->vm_end - vma->vm_start) != PAGE_SIZE) || vma->vm_pgoff)
 		return -EINVAL;
 
@@ -393,10 +411,13 @@ static int hpet_mmap(struct file *file, struct vm_area_struct *vma)
 	}
 
 	return 0;
+}
 #else
+static int hpet_mmap(struct file *file, struct vm_area_struct *vma)
+{
 	return -ENOSYS;
-#endif
 }
+#endif
 
 static int hpet_fasync(int fd, struct file *file, int on)
 {
-- 
1.7.9.3

Re: [PATCH] hpet, allow user controlled mmap for user processes

[Matt Wilson]

On Fri, Mar 22, 2013 at 09:32:54AM -0400, Prarit Bhargava wrote:
> The CONFIG_HPET_MMAP Kconfig option exposes the memory map of the HPET
> registers to userspace.  The Kconfig help points out that in some cases this
> can be a security risk as some systems may erroneously configure the map such
> that additional data is exposed to userspace.
> 
> This is a problem for distributions -- some users want the MMAP functionality
> but it comes with a significant security risk.  In an effort to mitigate this
> risk, and due to the low number of users of the MMAP functionality, I've
> introduced a kernel parameter, hpet_mmap_enable, that is required in order
> to actually have the HPET MMAP exposed.
> 
> [v2]: Clemens suggested modifying the Kconfig help text and making the
> default setting configurable.
> [v3]: Fixed up Documentation and Kconfig entries, default now "Y"
> [v4]: After testing, found that I need to modify CONFIG_HPET_MMAP_DEFAULT usage
> 
> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
> Cc: Clemens Ladisch <clemens@ladisch.de>
> ---
>  Documentation/kernel-parameters.txt |    4 ++++
>  drivers/char/Kconfig                |    9 +++++++--
>  drivers/char/hpet.c                 |   25 +++++++++++++++++++++++--
>  3 files changed, 34 insertions(+), 4 deletions(-)

It doesn't seem like this patch got picked up and seems like a good
idea to me. Clemens, what do you think?

Acked-by: Matt Wilson <msw@amazon.com>

--msw

Re: [PATCH] hpet, allow user controlled mmap for user processes

[Prarit Bhargava]

On 08/29/2013 02:01 AM, Matt Wilson wrote:
> On Fri, Mar 22, 2013 at 09:32:54AM -0400, Prarit Bhargava wrote:
>> The CONFIG_HPET_MMAP Kconfig option exposes the memory map of the HPET
>> registers to userspace.  The Kconfig help points out that in some cases this
>> can be a security risk as some systems may erroneously configure the map such
>> that additional data is exposed to userspace.
>>
>> This is a problem for distributions -- some users want the MMAP functionality
>> but it comes with a significant security risk.  In an effort to mitigate this
>> risk, and due to the low number of users of the MMAP functionality, I've
>> introduced a kernel parameter, hpet_mmap_enable, that is required in order
>> to actually have the HPET MMAP exposed.
>>
>> [v2]: Clemens suggested modifying the Kconfig help text and making the
>> default setting configurable.
>> [v3]: Fixed up Documentation and Kconfig entries, default now "Y"
>> [v4]: After testing, found that I need to modify CONFIG_HPET_MMAP_DEFAULT usage
>>
>> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
>> Cc: Clemens Ladisch <clemens@ladisch.de>
>> ---
>>  Documentation/kernel-parameters.txt |    4 ++++
>>  drivers/char/Kconfig                |    9 +++++++--
>>  drivers/char/hpet.c                 |   25 +++++++++++++++++++++++--
>>  3 files changed, 34 insertions(+), 4 deletions(-)
> 
> It doesn't seem like this patch got picked up and seems like a good
> idea to me. Clemens, what do you think?
> 
> Acked-by: Matt Wilson <msw@amazon.com>
> 

Clemens?  I didn't see a reply...

P.

[PATCH] hpet: allow user controlled mmap for user processes

[Clemens Ladisch]

From: Prarit Bhargava <prarit@redhat.com>

The CONFIG_HPET_MMAP Kconfig option exposes the memory map of the HPET
registers to userspace.  The Kconfig help points out that in some cases this
can be a security risk as some systems may erroneously configure the map such
that additional data is exposed to userspace.

This is a problem for distributions -- some users want the MMAP functionality
but it comes with a significant security risk.  In an effort to mitigate this
risk, and due to the low number of users of the MMAP functionality, I've
introduced a kernel parameter, hpet_mmap_enable, that is required in order
to actually have the HPET MMAP exposed.

[v2]: Clemens suggested modifying the Kconfig help text and making the
      default setting configurable.
[v3]: Fixed up Documentation and Kconfig entries, default now "Y"
[v4]: After testing, found that I need to modify CONFIG_HPET_MMAP_DEFAULT usage
[v5]: Fixed up Documentation, Kconfig entry, and log message [CL]

Signed-off-by: Prarit Bhargava <prarit@redhat.com>
Acked-by: Matt Wilson <msw@amazon.com>
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
---
 Documentation/kernel-parameters.txt |    3 +++
 drivers/char/Kconfig                |   10 ++++++++--
 drivers/char/hpet.c                 |   24 ++++++++++++++++++++++--
 3 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 539a236..6a7b656 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1064,6 +1064,9 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 				VIA, nVidia)
 			verbose: show contents of HPET registers during setup

+	hpet_mmap=	[X86, HPET_MMAP] Allow userspace to mmap HPET
+			registers.  Default set by CONFIG_HPET_MMAP_DEFAULT.
+
 	hugepages=	[HW,X86-32,IA-64] HugeTLB pages to allocate at boot.
 	hugepagesz=	[HW,IA-64,PPC,X86-64] The size of the HugeTLB pages.
 			On x86-64 and powerpc, this option can be specified
diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig
index 1421997..fa3243d 100644
--- a/drivers/char/Kconfig
+++ b/drivers/char/Kconfig
@@ -522,10 +522,16 @@ config HPET_MMAP
 	  If you say Y here, user applications will be able to mmap
 	  the HPET registers.

+config HPET_MMAP_DEFAULT
+	bool "Enable HPET MMAP access by default"
+	default y
+	depends on HPET_MMAP
+	help
 	  In some hardware implementations, the page containing HPET
 	  registers may also contain other things that shouldn't be
-	  exposed to the user.  If this applies to your hardware,
-	  say N here.
+	  exposed to the user.  This option selects the default (if
+	  kernel parameter hpet_mmap is not set) user access to the
+	  registers for applications that require it.

 config HANGCHECK_TIMER
 	tristate "Hangcheck timer"
diff --git a/drivers/char/hpet.c b/drivers/char/hpet.c
index d6568a6..964d002 100644
--- a/drivers/char/hpet.c
+++ b/drivers/char/hpet.c
@@ -367,12 +367,29 @@ static unsigned int hpet_poll(struct file *file, poll_table * wait)
 	return 0;
 }

+#ifdef CONFIG_HPET_MMAP
+#ifdef CONFIG_HPET_MMAP_DEFAULT
+static int hpet_mmap_enabled = 1;
+#else
+static int hpet_mmap_enabled = 0;
+#endif
+
+static __init int hpet_mmap_enable(char *str)
+{
+	get_option(&str, &hpet_mmap_enabled);
+	pr_info("HPET mmap %s\n", hpet_mmap_enabled ? "enabled" : "disabled");
+	return 1;
+}
+__setup("hpet_mmap", hpet_mmap_enable);
+
 static int hpet_mmap(struct file *file, struct vm_area_struct *vma)
 {
-#ifdef	CONFIG_HPET_MMAP
 	struct hpet_dev *devp;
 	unsigned long addr;

+	if (!hpet_mmap_enabled)
+		return -EACCES;
+
 	devp = file->private_data;
 	addr = devp->hd_hpets->hp_hpet_phys;

@@ -381,10 +398,13 @@ static int hpet_mmap(struct file *file, struct vm_area_struct *vma)

 	vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
 	return vm_iomap_memory(vma, addr, PAGE_SIZE);
+}
 #else
+static int hpet_mmap(struct file *file, struct vm_area_struct *vma)
+{
 	return -ENOSYS;
-#endif
 }
+#endif

 static int hpet_fasync(int fd, struct file *file, int on)
 {

Re: [PATCH] hpet, allow user controlled mmap for user processes

[Prarit Bhargava]

The CONFIG_HPET_MMAP Kconfig option exposes the memory map of the HPET
registers to userspace.  The Kconfig help points out that in some cases this
can be a security risk as some systems may erroneously configure the map such
that additional data is exposed to userspace.

This is a problem for distributions -- some users want the MMAP functionality
but it comes with a significant security risk.  In an effort to mitigate this
risk, and due to the low number of users of the MMAP functionality, I've
introduced a kernel parameter, hpet_mmap_enable, that is required in order
to actually have the HPET MMAP exposed.

[v2]: Clemens suggested modifying the Kconfig help text and making the
default setting configurable.
[v3]: Fixed up Documentation and Kconfig entries, default now "Y"

Signed-off-by: Prarit Bhargava <prarit@redhat.com>
Cc: Clemens Ladisch <clemens@ladisch.de>
---
 Documentation/kernel-parameters.txt |  4 ++++
 drivers/char/Kconfig                |  9 +++++++--
 drivers/char/hpet.c                 | 21 +++++++++++++++++++--
 3 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index e567af3..1444491 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -962,6 +962,10 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 				VIA, nVidia)
 			verbose: show contents of HPET registers during setup
 
+	hpet_mmap=	[X86, HPET_MMAP] option to expose HPET MMAP to
+			userspace.  By default this is disabled. Values are
+			0(disabled) or 1(enabled).
+
 	hugepages=	[HW,X86-32,IA-64] HugeTLB pages to allocate at boot.
 	hugepagesz=	[HW,IA-64,PPC,X86-64] The size of the HugeTLB pages.
 			On x86-64 and powerpc, this option can be specified
diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig
index 3bb6fa3..51b62a1 100644
--- a/drivers/char/Kconfig
+++ b/drivers/char/Kconfig
@@ -534,10 +534,15 @@ config HPET_MMAP
 	  If you say Y here, user applications will be able to mmap
 	  the HPET registers.
 
+config HPET_MMAP_DEFAULT
+	bool "Enable HPET MMAP access by default"
+	default y
+	depends on HPET_MMAP
+	help
 	  In some hardware implementations, the page containing HPET
 	  registers may also contain other things that shouldn't be
-	  exposed to the user.  If this applies to your hardware,
-	  say N here.
+	  exposed to the user. This option selects the default user access
+	  to the HPET registers for applications that require it.
 
 config HANGCHECK_TIMER
 	tristate "Hangcheck timer"
diff --git a/drivers/char/hpet.c b/drivers/char/hpet.c
index e3f9a99..0f32e30 100644
--- a/drivers/char/hpet.c
+++ b/drivers/char/hpet.c
@@ -367,12 +367,26 @@ static unsigned int hpet_poll(struct file *file, poll_table * wait)
 	return 0;
 }
 
+#ifdef CONFIG_HPET_MMAP
+static int hpet_mmap_enabled = CONFIG_HPET_MMAP_DEFAULT;
+
+static __init int hpet_mmap_enable(char *str)
+{
+	get_option(&str, &hpet_mmap_enabled);
+	pr_info(KERN_INFO "HPET MMAP %s\n",
+		hpet_mmap_enabled ? "disabled" : "enabled");
+	return 1;
+}
+__setup("hpet_mmap", hpet_mmap_enable);
+
 static int hpet_mmap(struct file *file, struct vm_area_struct *vma)
 {
-#ifdef	CONFIG_HPET_MMAP
 	struct hpet_dev *devp;
 	unsigned long addr;
 
+	if (!hpet_mmap_enabled)
+		return -EACCES;
+
 	if (((vma->vm_end - vma->vm_start) != PAGE_SIZE) || vma->vm_pgoff)
 		return -EINVAL;
 
@@ -393,10 +407,13 @@ static int hpet_mmap(struct file *file, struct vm_area_struct *vma)
 	}
 
 	return 0;
+}
 #else
+static int hpet_mmap(struct file *file, struct vm_area_struct *vma)
+{
 	return -ENOSYS;
-#endif
 }
+#endif
 
 static int hpet_fasync(int fd, struct file *file, int on)
 {
-- 
1.8.1