[RESEND,RFC] kernel/ksysfs.c: restrict /sys/kernel/notes to root access

  From: Guixiong Wei <weiguixiong@bytedance.com>

Restrict non-privileged user access to /sys/kernel/notes to
avoid security attack.

The non-privileged users have read access to notes. The notes
expose the load address of startup_xen. This address could be
used to bypass KASLR.

For example, the startup_xen is built at 0xffffffff82465180 and
commit_creds is built at 0xffffffff810ad570 which could read from
the /boot/System.map. And the loaded address of startup_xen is
0xffffffffbc265180 which read from /sys/kernel/notes. So the loaded
address of commit_creds is 0xffffffffbc265180 - (0xffffffff82465180
 - 0xffffffff810ad570) = 0xffffffffbaead570.

Signed-off-by: Guixiong Wei <weiguixiong@bytedance.com>
---
 kernel/ksysfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

On Sun, Feb 18, 2024 at 03:35:01PM +0800, Guixiong Wei wrote:
> From: Guixiong Wei <weiguixiong@bytedance.com>
> 
> Restrict non-privileged user access to /sys/kernel/notes to
> avoid security attack.
> 
> The non-privileged users have read access to notes. The notes
> expose the load address of startup_xen. This address could be
> used to bypass KASLR.

How can it be used to bypass it?

KASLR is, for local users, pretty much not an issue, as that's not what
it protects from, only remote ones.

> For example, the startup_xen is built at 0xffffffff82465180 and
> commit_creds is built at 0xffffffff810ad570 which could read from
> the /boot/System.map. And the loaded address of startup_xen is
> 0xffffffffbc265180 which read from /sys/kernel/notes. So the loaded
> address of commit_creds is 0xffffffffbc265180 - (0xffffffff82465180
>  - 0xffffffff810ad570) = 0xffffffffbaead570.

I've cc: the hardening list on this, I'm sure the developers there have
opinions about this.

> Signed-off-by: Guixiong Wei <weiguixiong@bytedance.com>
> ---
>  kernel/ksysfs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/ksysfs.c b/kernel/ksysfs.c
> index b1292a57c2a5..09bc0730239b 100644
> --- a/kernel/ksysfs.c
> +++ b/kernel/ksysfs.c
> @@ -199,7 +199,7 @@ static ssize_t notes_read(struct file *filp, struct kobject *kobj,
>  static struct bin_attribute notes_attr __ro_after_init  = {
>  	.attr = {
>  		.name = "notes",
> -		.mode = S_IRUGO,
> +		.mode = S_IRUSR,
>  	},
>  	.read = &notes_read,
>  };

No objection from me, but what userspace tool requires access to this
file today?  Will it break if permissions are changed on it?

And what about the module notes files?  If you change one, shouldn't you
change all?

thanks,

greg k-h
  

On Sun, Feb 18, 2024 at 08:47:03AM +0100, Greg KH wrote:
> On Sun, Feb 18, 2024 at 03:35:01PM +0800, Guixiong Wei wrote:
> > From: Guixiong Wei <weiguixiong@bytedance.com>
> > 
> > Restrict non-privileged user access to /sys/kernel/notes to
> > avoid security attack.
> > 
> > The non-privileged users have read access to notes. The notes
> > expose the load address of startup_xen. This address could be
> > used to bypass KASLR.
> 
> How can it be used to bypass it?
> 
> KASLR is, for local users, pretty much not an issue, as that's not what
> it protects from, only remote ones.
> 
> > For example, the startup_xen is built at 0xffffffff82465180 and
> > commit_creds is built at 0xffffffff810ad570 which could read from
> > the /boot/System.map. And the loaded address of startup_xen is
> > 0xffffffffbc265180 which read from /sys/kernel/notes. So the loaded
> > address of commit_creds is 0xffffffffbc265180 - (0xffffffff82465180
> >  - 0xffffffff810ad570) = 0xffffffffbaead570.
> 
> I've cc: the hardening list on this, I'm sure the developers there have
> opinions about this.

Oh eww, why is Xen spewing addresses into the notes section? (This must
be how it finds its entry point? But that would be before relocations
happen...)

But yes, I can confirm that relocations are done against the .notes
section at boot, so the addresses exposed in .notes is an immediate
KASLR offset exposure.

In /sys/kernel/notes (are there any tools to read this? I wrote my own...)

	type: 1
	name: Xen
	desc: 0xb4a711c0 0xffffffff

which matches a privileged read of /proc/kallsysms:

	ffffffffb4a711c0 T startup_xen

(and the hypercall_page too)

There are all coming from arch/x86/xen/xen-head.S:

        ELFNOTE(Xen, XEN_ELFNOTE_GUEST_OS,       .asciz "linux")
        ELFNOTE(Xen, XEN_ELFNOTE_GUEST_VERSION,  .asciz "2.6")
        ELFNOTE(Xen, XEN_ELFNOTE_XEN_VERSION,    .asciz "xen-3.0")
#ifdef CONFIG_XEN_PV
        ELFNOTE(Xen, XEN_ELFNOTE_VIRT_BASE,      _ASM_PTR __START_KERNEL_map)
        /* Map the p2m table to a 512GB-aligned user address. */
        ELFNOTE(Xen, XEN_ELFNOTE_INIT_P2M,       .quad (PUD_SIZE * PTRS_PER_PUD))
        ELFNOTE(Xen, XEN_ELFNOTE_ENTRY,          _ASM_PTR startup_xen)
..

Introduced in commit 5ead97c84fa7 ("xen: Core Xen implementation")

Exposed in commit da1a679cde9b ("Add /sys/kernel/notes")

Amazingly these both went in on the same release (v2.6.23, 2007). This
has been exposed for longer than KASLR has been upstream. :P

> 
> > Signed-off-by: Guixiong Wei <weiguixiong@bytedance.com>
> > ---
> >  kernel/ksysfs.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/kernel/ksysfs.c b/kernel/ksysfs.c
> > index b1292a57c2a5..09bc0730239b 100644
> > --- a/kernel/ksysfs.c
> > +++ b/kernel/ksysfs.c
> > @@ -199,7 +199,7 @@ static ssize_t notes_read(struct file *filp, struct kobject *kobj,
> >  static struct bin_attribute notes_attr __ro_after_init  = {
> >  	.attr = {
> >  		.name = "notes",
> > -		.mode = S_IRUGO,
> > +		.mode = S_IRUSR,
> >  	},
> >  	.read = &notes_read,
> >  };

Yes please.

Reviewed-by: Kees Cook <keescook@chromium.org>

I wonder if we should also remove relocations that are aimed at the
notes section for good measure? If that had already been true, this
would have just given the same info as System.map.

> 
> No objection from me, but what userspace tool requires access to this
> file today?  Will it break if permissions are changed on it?
> 
> And what about the module notes files?  If you change one, shouldn't you
> change all?

Luckily all of _those_ contain what I'd expect: the Linux and
GNU.build-id notes, which are harmless. But if we're going to suddenly
have things appearing in here, let's make those root-only too.

-Kees
  

On 2024/2/18 17:04, Kees Cook wrote:
> On Sun, Feb 18, 2024 at 08:47:03AM +0100, Greg KH wrote:
>> On Sun, Feb 18, 2024 at 03:35:01PM +0800, Guixiong Wei wrote:
>>> From: Guixiong Wei <weiguixiong@bytedance.com>
>>>
>>> Restrict non-privileged user access to /sys/kernel/notes to
>>> avoid security attack.
>>>
>>> The non-privileged users have read access to notes. The notes
>>> expose the load address of startup_xen. This address could be
>>> used to bypass KASLR.
>> How can it be used to bypass it?
>>
>> KASLR is, for local users, pretty much not an issue, as that's not what
>> it protects from, only remote ones.
>>
>>> For example, the startup_xen is built at 0xffffffff82465180 and
>>> commit_creds is built at 0xffffffff810ad570 which could read from
>>> the /boot/System.map. And the loaded address of startup_xen is
>>> 0xffffffffbc265180 which read from /sys/kernel/notes. So the loaded
>>> address of commit_creds is 0xffffffffbc265180 - (0xffffffff82465180
>>>   - 0xffffffff810ad570) = 0xffffffffbaead570.
>> I've cc: the hardening list on this, I'm sure the developers there have
>> opinions about this.
> Oh eww, why is Xen spewing addresses into the notes section? (This must
> be how it finds its entry point? But that would be before relocations
> happen...)
>
> But yes, I can confirm that relocations are done against the .notes
> section at boot, so the addresses exposed in .notes is an immediate
> KASLR offset exposure.
>
> In /sys/kernel/notes (are there any tools to read this? I wrote my own...)
>
> 	type: 1
> 	name: Xen
> 	desc: 0xb4a711c0 0xffffffff
>
> which matches a privileged read of /proc/kallsysms:
>
> 	ffffffffb4a711c0 T startup_xen
>
> (and the hypercall_page too)
>
> There are all coming from arch/x86/xen/xen-head.S:
>
>          ELFNOTE(Xen, XEN_ELFNOTE_GUEST_OS,       .asciz "linux")
>          ELFNOTE(Xen, XEN_ELFNOTE_GUEST_VERSION,  .asciz "2.6")
>          ELFNOTE(Xen, XEN_ELFNOTE_XEN_VERSION,    .asciz "xen-3.0")
> #ifdef CONFIG_XEN_PV
>          ELFNOTE(Xen, XEN_ELFNOTE_VIRT_BASE,      _ASM_PTR __START_KERNEL_map)
>          /* Map the p2m table to a 512GB-aligned user address. */
>          ELFNOTE(Xen, XEN_ELFNOTE_INIT_P2M,       .quad (PUD_SIZE * PTRS_PER_PUD))
>          ELFNOTE(Xen, XEN_ELFNOTE_ENTRY,          _ASM_PTR startup_xen)
> ...
>
> Introduced in commit 5ead97c84fa7 ("xen: Core Xen implementation")
>
> Exposed in commit da1a679cde9b ("Add /sys/kernel/notes")
>
> Amazingly these both went in on the same release (v2.6.23, 2007). This
> has been exposed for longer than KASLR has been upstream. :P
>
>>> Signed-off-by: Guixiong Wei <weiguixiong@bytedance.com>
>>> ---
>>>   kernel/ksysfs.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/kernel/ksysfs.c b/kernel/ksysfs.c
>>> index b1292a57c2a5..09bc0730239b 100644
>>> --- a/kernel/ksysfs.c
>>> +++ b/kernel/ksysfs.c
>>> @@ -199,7 +199,7 @@ static ssize_t notes_read(struct file *filp, struct kobject *kobj,
>>>   static struct bin_attribute notes_attr __ro_after_init  = {
>>>   	.attr = {
>>>   		.name = "notes",
>>> -		.mode = S_IRUGO,
>>> +		.mode = S_IRUSR,
>>>   	},
>>>   	.read = &notes_read,
>>>   };
> Yes please.
>
> Reviewed-by: Kees Cook <keescook@chromium.org>
>
> I wonder if we should also remove relocations that are aimed at the
> .notes section for good measure? If that had already been true, this
> would have just given the same info as System.map.
That's a good idea, but it depends on whether the user space tool can 
accept the remove relocation address.
>> No objection from me, but what userspace tool requires access to this
>> file today?  Will it break if permissions are changed on it?
 From the exposed content, it seems that the main users are Xen-related 
tools. I add Xen list, developers should be able to provide some 
information.
>> And what about the module notes files?  If you change one, shouldn't you
>> change all?
 From what I currently know, the module note files do not expose any 
kernel symbol address, so there is no need for modification.
> Luckily all of _those_ contain what I'd expect: the Linux and
> GNU.build-id notes, which are harmless. But if we're going to suddenly
> have things appearing in here, let's make those root-only too.
Yes, but I also not sure whether the user space tools using this file 
can accept this permission modification.
  

On 18.02.24 10:04, Kees Cook wrote:
> On Sun, Feb 18, 2024 at 08:47:03AM +0100, Greg KH wrote:
>> On Sun, Feb 18, 2024 at 03:35:01PM +0800, Guixiong Wei wrote:
>>> From: Guixiong Wei <weiguixiong@bytedance.com>
>>>
>>> Restrict non-privileged user access to /sys/kernel/notes to
>>> avoid security attack.
>>>
>>> The non-privileged users have read access to notes. The notes
>>> expose the load address of startup_xen. This address could be
>>> used to bypass KASLR.
>>
>> How can it be used to bypass it?
>>
>> KASLR is, for local users, pretty much not an issue, as that's not what
>> it protects from, only remote ones.
>>
>>> For example, the startup_xen is built at 0xffffffff82465180 and
>>> commit_creds is built at 0xffffffff810ad570 which could read from
>>> the /boot/System.map. And the loaded address of startup_xen is
>>> 0xffffffffbc265180 which read from /sys/kernel/notes. So the loaded
>>> address of commit_creds is 0xffffffffbc265180 - (0xffffffff82465180
>>>   - 0xffffffff810ad570) = 0xffffffffbaead570.
>>
>> I've cc: the hardening list on this, I'm sure the developers there have
>> opinions about this.
> 
> Oh eww, why is Xen spewing addresses into the notes section? (This must
> be how it finds its entry point? But that would be before relocations
> happen...)

Right. Xen is looking into the ELF-file to find the entry point of the
kernel (PV and PVH guest types only).

> 
> But yes, I can confirm that relocations are done against the .notes
> section at boot, so the addresses exposed in .notes is an immediate
> KASLR offset exposure.

Relocations applied to the kernel when it has been started don't need
to cover the notes section as far as Xen is concerned.


Juergen
  

On Sun, Feb 18, 2024 at 8:47 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Sun, Feb 18, 2024 at 03:35:01PM +0800, Guixiong Wei wrote:
> > From: Guixiong Wei <weiguixiong@bytedance.com>
> >
> > Restrict non-privileged user access to /sys/kernel/notes to
> > avoid security attack.
> >
> > The non-privileged users have read access to notes. The notes
> > expose the load address of startup_xen. This address could be
> > used to bypass KASLR.
>
> How can it be used to bypass it?
>
> KASLR is, for local users, pretty much not an issue, as that's not what
> it protects from, only remote ones.
>
> > For example, the startup_xen is built at 0xffffffff82465180 and
> > commit_creds is built at 0xffffffff810ad570 which could read from
> > the /boot/System.map. And the loaded address of startup_xen is
> > 0xffffffffbc265180 which read from /sys/kernel/notes. So the loaded
> > address of commit_creds is 0xffffffffbc265180 - (0xffffffff82465180
> >  - 0xffffffff810ad570) = 0xffffffffbaead570.
>
> I've cc: the hardening list on this, I'm sure the developers there have
> opinions about this.
>
> > Signed-off-by: Guixiong Wei <weiguixiong@bytedance.com>
> > ---
> >  kernel/ksysfs.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/kernel/ksysfs.c b/kernel/ksysfs.c
> > index b1292a57c2a5..09bc0730239b 100644
> > --- a/kernel/ksysfs.c
> > +++ b/kernel/ksysfs.c
> > @@ -199,7 +199,7 @@ static ssize_t notes_read(struct file *filp, struct kobject *kobj,
> >  static struct bin_attribute notes_attr __ro_after_init  = {
> >       .attr = {
> >               .name = "notes",
> > -             .mode = S_IRUGO,
> > +             .mode = S_IRUSR,
> >       },
> >       .read = &notes_read,
> >  };
>
> No objection from me, but what userspace tool requires access to this
> file today?  Will it break if permissions are changed on it?

FWIW, https://codesearch.debian.net/search?q=%2Fsys%2Fkernel%2Fnotes&literal=1
shows that (from what I can tell) this is mostly used by stuff like
perf, libdwfl and systemtap for figuring out the kernel's build-id,
probably mostly for kernel profiling?