* Re: [PATCH 1/1] Ddrivers: hv: Turn off write permission on the hypercall page
[not found] <1486603856-27642-1-git-send-email-kys@exchange.microsoft.com>
@ 2017-02-09 4:44 ` Kees Cook
2017-02-09 6:54 ` Greg KH
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Kees Cook @ 2017-02-09 4:44 UTC (permalink / raw)
To: K. Y. Srinivasan
Cc: Greg KH, LKML, devel, Olaf Hering, Andy Whitcroft,
Vitaly Kuznetsov, Jason Wang, Leann Ogasawara, Stephen Hemminger,
Stephen Smalley, x86@kernel.org, Laura Abbott,
kernel-hardening@lists.openwall.com
On Wed, Feb 8, 2017 at 5:30 PM, <kys@exchange.microsoft.com> wrote:
> From: K. Y. Srinivasan <kys@microsoft.com>
>
> The hypercall page only needs to be executable but currently it is setup to
> be writable as well. Fix the issue.
>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> Cc: <stable@vger.kernel.org>
> ---
> arch/x86/hyperv/hv_init.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> index c224b7d..db64baf 100644
> --- a/arch/x86/hyperv/hv_init.c
> +++ b/arch/x86/hyperv/hv_init.c
> @@ -125,7 +125,7 @@ void hyperv_init(void)
> guest_id = generate_guest_id(0, LINUX_VERSION_CODE, 0);
> wrmsrl(HV_X64_MSR_GUEST_OS_ID, guest_id);
>
> - hypercall_pg = __vmalloc(PAGE_SIZE, GFP_KERNEL, PAGE_KERNEL_EXEC);
> + hypercall_pg = __vmalloc(PAGE_SIZE, GFP_KERNEL, PAGE_KERNEL_RX);
> if (hypercall_pg == NULL) {
> wrmsrl(HV_X64_MSR_GUEST_OS_ID, 0);
> return;
> --
> 1.7.1
>
Acked-by: Kees Cook <keescook@chromium.org>
This does beg the question: why is PAGE_KERNEL_EXEC defined at all?
The only permissions available should be _RO, _RW, or _RX... Current
users in arch/x86:
kernel/machine_kexec_32.c: set_pte(pte, pfn_pte(paddr >>
PAGE_SHIFT, PAGE_KERNEL_EXEC));
kernel/machine_kexec_64.c: set_pte(pte, pfn_pte(paddr >>
PAGE_SHIFT, PAGE_KERNEL_EXEC));
kernel/module.c: PAGE_KERNEL_EXEC,
0, NUMA_NO_NODE,
kernel/tboot.c: if (map_tboot_page(vaddr, start_pfn, PAGE_KERNEL_EXEC))
mm/init_32.c: prot = PAGE_KERNEL_EXEC;
power/hibernate_32.c: set_pte(pte,
pfn_pte(pfn, PAGE_KERNEL_EXEC));
xen/mmu.c: pte = pfn_pte(pfn, PAGE_KERNEL_EXEC);
and vmalloc_exec...
void *vmalloc_exec(unsigned long size)
{
return __vmalloc_node(size, 1, GFP_KERNEL | __GFP_HIGHMEM,
PAGE_KERNEL_EXEC,
NUMA_NO_NODE, __builtin_return_address(0));
}
which is only ever used by module_alloc(), and the module code knows
how to make things RW, RO, and RX correctly. (Though on x86,
module_alloc() is overridden, but it still uses PAGE_KERNEL_EXEC.)
Seems like x86 should drop PAGE_KERNEL_EXEC, and vmalloc_exec should
use PAGE_KERNEL... ?
Or better yet, drop PAGE_KERNEL_EXEC globally and rename PAGE_KERNEL
to PAGE_KERNEL_RW.
-Kees
--
Kees Cook
Pixel Security
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/1] Ddrivers: hv: Turn off write permission on the hypercall page
[not found] <1486603856-27642-1-git-send-email-kys@exchange.microsoft.com>
2017-02-09 4:44 ` [PATCH 1/1] Ddrivers: hv: Turn off write permission on the hypercall page Kees Cook
@ 2017-02-09 6:54 ` Greg KH
2017-02-09 15:48 ` KY Srinivasan
2017-02-10 1:12 ` Stephen Hemminger
2017-02-10 14:47 ` Greg KH
3 siblings, 1 reply; 6+ messages in thread
From: Greg KH @ 2017-02-09 6:54 UTC (permalink / raw)
To: kys
Cc: linux-kernel, devel, olaf, apw, vkuznets, jasowang,
leann.ogasawara, keescook, stephen, sds, stable
On Wed, Feb 08, 2017 at 06:30:56PM -0700, kys@exchange.microsoft.com wrote:
> From: K. Y. Srinivasan <kys@microsoft.com>
>
> The hypercall page only needs to be executable but currently it is setup to
> be writable as well. Fix the issue.
>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> Cc: <stable@vger.kernel.org>
What type of bug is this fixing that it is needed to be backported?
Does this affect any current users? How far back should it go?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread* RE: [PATCH 1/1] Ddrivers: hv: Turn off write permission on the hypercall page
2017-02-09 6:54 ` Greg KH
@ 2017-02-09 15:48 ` KY Srinivasan
0 siblings, 0 replies; 6+ messages in thread
From: KY Srinivasan @ 2017-02-09 15:48 UTC (permalink / raw)
To: Greg KH
Cc: linux-kernel@vger.kernel.org, devel@linuxdriverproject.org,
olaf@aepfle.de, apw@canonical.com, vkuznets@redhat.com,
jasowang@redhat.com, leann.ogasawara@canonical.com,
keescook@google.com, stephen@networkplumber.org,
sds@tycho.nsa.gov, stable@vger.kernel.org
> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Wednesday, February 8, 2017 10:55 PM
> To: KY Srinivasan <kys@microsoft.com>
> Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> olaf@aepfle.de; apw@canonical.com; vkuznets@redhat.com;
> jasowang@redhat.com; leann.ogasawara@canonical.com;
> keescook@google.com; stephen@networkplumber.org; sds@tycho.nsa.gov;
> stable@vger.kernel.org
> Subject: Re: [PATCH 1/1] Ddrivers: hv: Turn off write permission on the
> hypercall page
>
> On Wed, Feb 08, 2017 at 06:30:56PM -0700, kys@exchange.microsoft.com
> wrote:
> > From: K. Y. Srinivasan <kys@microsoft.com>
> >
> > The hypercall page only needs to be executable but currently it is setup to
> > be writable as well. Fix the issue.
> >
> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > Cc: <stable@vger.kernel.org>
>
> What type of bug is this fixing that it is needed to be backported?
> Does this affect any current users? How far back should it go?
This issue has been there from day 1 and the mapping was setup after the
scan was done (for questionable mappings) and so we did not see it. Now, we
are setting up the hypercall page much earlier in the boot sequence and so
this issue was noticed. Once this is committed, I will send the patch for stable.
The main issue is that the page can be written and is executable and could be
a vulnerability.
Thanks,
K. Y
>
> thanks,
>
> greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] Ddrivers: hv: Turn off write permission on the hypercall page
[not found] <1486603856-27642-1-git-send-email-kys@exchange.microsoft.com>
2017-02-09 4:44 ` [PATCH 1/1] Ddrivers: hv: Turn off write permission on the hypercall page Kees Cook
2017-02-09 6:54 ` Greg KH
@ 2017-02-10 1:12 ` Stephen Hemminger
2017-02-10 14:47 ` Greg KH
3 siblings, 0 replies; 6+ messages in thread
From: Stephen Hemminger @ 2017-02-10 1:12 UTC (permalink / raw)
To: kys
Cc: kys, gregkh, linux-kernel, devel, olaf, apw, vkuznets, jasowang,
leann.ogasawara, keescook, sds, stable
On Wed, 8 Feb 2017 18:30:56 -0700
kys@exchange.microsoft.com wrote:
> From: K. Y. Srinivasan <kys@microsoft.com>
>
> The hypercall page only needs to be executable but currently it is setup to
> be writable as well. Fix the issue.
>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> Cc: <stable@vger.kernel.org>
> ---
> arch/x86/hyperv/hv_init.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> index c224b7d..db64baf 100644
> --- a/arch/x86/hyperv/hv_init.c
> +++ b/arch/x86/hyperv/hv_init.c
> @@ -125,7 +125,7 @@ void hyperv_init(void)
> guest_id = generate_guest_id(0, LINUX_VERSION_CODE, 0);
> wrmsrl(HV_X64_MSR_GUEST_OS_ID, guest_id);
>
> - hypercall_pg = __vmalloc(PAGE_SIZE, GFP_KERNEL, PAGE_KERNEL_EXEC);
> + hypercall_pg = __vmalloc(PAGE_SIZE, GFP_KERNEL, PAGE_KERNEL_RX);
> if (hypercall_pg == NULL) {
> wrmsrl(HV_X64_MSR_GUEST_OS_ID, 0);
Verified that this fixes the original problem.
I don't see the warning anymore and the guest works normally.
Reported-by: Stephen Hemminger <stephen@networkplumber.org>
Tested-by: Stephen Hemminger <stephen@networkplumber.org>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/1] Ddrivers: hv: Turn off write permission on the hypercall page
[not found] <1486603856-27642-1-git-send-email-kys@exchange.microsoft.com>
` (2 preceding siblings ...)
2017-02-10 1:12 ` Stephen Hemminger
@ 2017-02-10 14:47 ` Greg KH
2017-02-10 15:15 ` KY Srinivasan
3 siblings, 1 reply; 6+ messages in thread
From: Greg KH @ 2017-02-10 14:47 UTC (permalink / raw)
To: kys
Cc: linux-kernel, devel, olaf, apw, vkuznets, jasowang,
leann.ogasawara, keescook, stephen, sds, stable
On Wed, Feb 08, 2017 at 06:30:56PM -0700, kys@exchange.microsoft.com wrote:
> From: K. Y. Srinivasan <kys@microsoft.com>
>
> The hypercall page only needs to be executable but currently it is setup to
> be writable as well. Fix the issue.
>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
I had to hand-edit the subject: line, please be more careful :(
^ permalink raw reply [flat|nested] 6+ messages in thread* RE: [PATCH 1/1] Ddrivers: hv: Turn off write permission on the hypercall page
2017-02-10 14:47 ` Greg KH
@ 2017-02-10 15:15 ` KY Srinivasan
0 siblings, 0 replies; 6+ messages in thread
From: KY Srinivasan @ 2017-02-10 15:15 UTC (permalink / raw)
To: Greg KH
Cc: linux-kernel@vger.kernel.org, devel@linuxdriverproject.org,
olaf@aepfle.de, apw@canonical.com, vkuznets@redhat.com,
jasowang@redhat.com, leann.ogasawara@canonical.com,
keescook@google.com, stephen@networkplumber.org,
sds@tycho.nsa.gov, stable@vger.kernel.org
> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Friday, February 10, 2017 6:48 AM
> To: KY Srinivasan <kys@microsoft.com>
> Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> olaf@aepfle.de; apw@canonical.com; vkuznets@redhat.com;
> jasowang@redhat.com; leann.ogasawara@canonical.com;
> keescook@google.com; stephen@networkplumber.org; sds@tycho.nsa.gov;
> stable@vger.kernel.org
> Subject: Re: [PATCH 1/1] Ddrivers: hv: Turn off write permission on the
> hypercall page
>
> On Wed, Feb 08, 2017 at 06:30:56PM -0700, kys@exchange.microsoft.com
> wrote:
> > From: K. Y. Srinivasan <kys@microsoft.com>
> >
> > The hypercall page only needs to be executable but currently it is setup to
> > be writable as well. Fix the issue.
> >
> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
>
> I had to hand-edit the subject: line, please be more careful :(
Thanks; will do.
K. Y
^ permalink raw reply [flat|nested] 6+ messages in thread