Messages in this thread | | | Date | Mon, 19 Feb 2024 19:41:13 +0800 | Subject | Re: [RESEND RFC] kernel/ksysfs.c: restrict /sys/kernel/notes to root access | From | Guixiong Wei <> |
| |
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 = ¬es_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.
| |