To support finer grained kaslr (fgkaslr), we need to make a couple changes
to kallsyms. Firstly, we need to hide our sorted list of symbols, since
this will give away our new layout. Secondly, we will export the seed used
for randomizing the layout so that it can be used to make a particular
layout persist across boots for debug purposes.
Signed-off-by: Kristen Carlson Accardi <[email protected]>
---
kernel/kallsyms.c | 30 +++++++++++++++++++++++++++++-
1 file changed, 29 insertions(+), 1 deletion(-)
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index 136ce049c4ad..432b13a3a033 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -698,6 +698,21 @@ const char *kdb_walk_kallsyms(loff_t *pos)
}
#endif /* CONFIG_KGDB_KDB */
+#ifdef CONFIG_FG_KASLR
+extern const u64 fgkaslr_seed[] __weak;
+
+static int proc_fgkaslr_show(struct seq_file *m, void *v)
+{
+ seq_printf(m, "%llx\n", fgkaslr_seed[0]);
+ seq_printf(m, "%llx\n", fgkaslr_seed[1]);
+ seq_printf(m, "%llx\n", fgkaslr_seed[2]);
+ seq_printf(m, "%llx\n", fgkaslr_seed[3]);
+ return 0;
+}
+#else
+static inline int proc_fgkaslr_show(struct seq_file *m, void *v) { return 0; }
+#endif
+
static const struct file_operations kallsyms_operations = {
.open = kallsyms_open,
.read = seq_read,
@@ -707,7 +722,20 @@ static const struct file_operations kallsyms_operations = {
static int __init kallsyms_init(void)
{
- proc_create("kallsyms", 0444, NULL, &kallsyms_operations);
+ /*
+ * When fine grained kaslr is enabled, we don't want to
+ * print out the symbols even with zero pointers because
+ * this reveals the randomization order. If fg kaslr is
+ * enabled, make kallsyms available only to privileged
+ * users.
+ */
+ if (!IS_ENABLED(CONFIG_FG_KASLR))
+ proc_create("kallsyms", 0444, NULL, &kallsyms_operations);
+ else {
+ proc_create_single("fgkaslr_seed", 0400, NULL,
+ proc_fgkaslr_show);
+ proc_create("kallsyms", 0400, NULL, &kallsyms_operations);
+ }
return 0;
}
device_initcall(kallsyms_init);
--
2.24.1
On Wed, Feb 05, 2020 at 02:39:48PM -0800, Kristen Carlson Accardi wrote:
> To support finer grained kaslr (fgkaslr), we need to make a couple changes
> to kallsyms. Firstly, we need to hide our sorted list of symbols, since
> this will give away our new layout. Secondly, we will export the seed used
> for randomizing the layout so that it can be used to make a particular
> layout persist across boots for debug purposes.
>
> Signed-off-by: Kristen Carlson Accardi <[email protected]>
> ---
> kernel/kallsyms.c | 30 +++++++++++++++++++++++++++++-
> 1 file changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
> index 136ce049c4ad..432b13a3a033 100644
> --- a/kernel/kallsyms.c
> +++ b/kernel/kallsyms.c
> @@ -698,6 +698,21 @@ const char *kdb_walk_kallsyms(loff_t *pos)
> }
> #endif /* CONFIG_KGDB_KDB */
>
> +#ifdef CONFIG_FG_KASLR
> +extern const u64 fgkaslr_seed[] __weak;
> +
> +static int proc_fgkaslr_show(struct seq_file *m, void *v)
> +{
> + seq_printf(m, "%llx\n", fgkaslr_seed[0]);
> + seq_printf(m, "%llx\n", fgkaslr_seed[1]);
> + seq_printf(m, "%llx\n", fgkaslr_seed[2]);
> + seq_printf(m, "%llx\n", fgkaslr_seed[3]);
> + return 0;
> +}
> +#else
> +static inline int proc_fgkaslr_show(struct seq_file *m, void *v) { return 0; }
> +#endif
> +
I'd like to put the fgkaslr seed exposure behind a separate DEBUG
config, since it shouldn't be normally exposed. As such, its
infrastructure should be likely extracted from this and the main fgkaslr
patches and added back separately (and maybe it will entirely vanish
once the RNG is switched to ChaCha20).
> static const struct file_operations kallsyms_operations = {
> .open = kallsyms_open,
> .read = seq_read,
> @@ -707,7 +722,20 @@ static const struct file_operations kallsyms_operations = {
>
> static int __init kallsyms_init(void)
> {
> - proc_create("kallsyms", 0444, NULL, &kallsyms_operations);
> + /*
> + * When fine grained kaslr is enabled, we don't want to
> + * print out the symbols even with zero pointers because
> + * this reveals the randomization order. If fg kaslr is
> + * enabled, make kallsyms available only to privileged
> + * users.
> + */
> + if (!IS_ENABLED(CONFIG_FG_KASLR))
> + proc_create("kallsyms", 0444, NULL, &kallsyms_operations);
> + else {
> + proc_create_single("fgkaslr_seed", 0400, NULL,
> + proc_fgkaslr_show);
> + proc_create("kallsyms", 0400, NULL, &kallsyms_operations);
> + }
> return 0;
> }
> device_initcall(kallsyms_init);
> --
> 2.24.1
In the past, making kallsyms entirely unreadable seemed to break weird
stuff in userspace. How about having an alternative view that just
contains a alphanumeric sort of the symbol names (and they will continue
to have zeroed addresses for unprivileged users)?
Or perhaps we wait to hear about this causing a problem, and deal with
it then? :)
--
Kees Cook
On Thu, 2020-02-06 at 04:32 -0800, Kees Cook wrote:
> On Wed, Feb 05, 2020 at 02:39:48PM -0800, Kristen Carlson Accardi
> wrote:
> > To support finer grained kaslr (fgkaslr), we need to make a couple
> > changes
> > to kallsyms. Firstly, we need to hide our sorted list of symbols,
> > since
> > this will give away our new layout. Secondly, we will export the
> > seed used
> > for randomizing the layout so that it can be used to make a
> > particular
> > layout persist across boots for debug purposes.
> >
> > Signed-off-by: Kristen Carlson Accardi <[email protected]>
> > ---
> > kernel/kallsyms.c | 30 +++++++++++++++++++++++++++++-
> > 1 file changed, 29 insertions(+), 1 deletion(-)
> >
> > diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
> > index 136ce049c4ad..432b13a3a033 100644
> > --- a/kernel/kallsyms.c
> > +++ b/kernel/kallsyms.c
> > @@ -698,6 +698,21 @@ const char *kdb_walk_kallsyms(loff_t *pos)
> > }
> > #endif /* CONFIG_KGDB_KDB */
> >
> > +#ifdef CONFIG_FG_KASLR
> > +extern const u64 fgkaslr_seed[] __weak;
> > +
> > +static int proc_fgkaslr_show(struct seq_file *m, void *v)
> > +{
> > + seq_printf(m, "%llx\n", fgkaslr_seed[0]);
> > + seq_printf(m, "%llx\n", fgkaslr_seed[1]);
> > + seq_printf(m, "%llx\n", fgkaslr_seed[2]);
> > + seq_printf(m, "%llx\n", fgkaslr_seed[3]);
> > + return 0;
> > +}
> > +#else
> > +static inline int proc_fgkaslr_show(struct seq_file *m, void *v) {
> > return 0; }
> > +#endif
> > +
>
> I'd like to put the fgkaslr seed exposure behind a separate DEBUG
> config, since it shouldn't be normally exposed. As such, its
> infrastructure should be likely extracted from this and the main
> fgkaslr
> patches and added back separately (and maybe it will entirely vanish
> once the RNG is switched to ChaCha20).
OK, sounds reasonable to me.
>
> > static const struct file_operations kallsyms_operations = {
> > .open = kallsyms_open,
> > .read = seq_read,
> > @@ -707,7 +722,20 @@ static const struct file_operations
> > kallsyms_operations = {
> >
> > static int __init kallsyms_init(void)
> > {
> > - proc_create("kallsyms", 0444, NULL, &kallsyms_operations);
> > + /*
> > + * When fine grained kaslr is enabled, we don't want to
> > + * print out the symbols even with zero pointers because
> > + * this reveals the randomization order. If fg kaslr is
> > + * enabled, make kallsyms available only to privileged
> > + * users.
> > + */
> > + if (!IS_ENABLED(CONFIG_FG_KASLR))
> > + proc_create("kallsyms", 0444, NULL,
> > &kallsyms_operations);
> > + else {
> > + proc_create_single("fgkaslr_seed", 0400, NULL,
> > + proc_fgkaslr_show);
> > + proc_create("kallsyms", 0400, NULL,
> > &kallsyms_operations);
> > + }
> > return 0;
> > }
> > device_initcall(kallsyms_init);
> > --
> > 2.24.1
>
> In the past, making kallsyms entirely unreadable seemed to break
> weird
> stuff in userspace. How about having an alternative view that just
> contains a alphanumeric sort of the symbol names (and they will
> continue
> to have zeroed addresses for unprivileged users)?
>
> Or perhaps we wait to hear about this causing a problem, and deal
> with
> it then? :)
>
Yeah - I don't know what people want here. Clearly, we can't leave
kallsyms the way it is. Removing it entirely is a pretty fast way to
figure out how people use it though :).
On Thu, Feb 6, 2020 at 6:51 PM Kristen Carlson Accardi
<[email protected]> wrote:
> On Thu, 2020-02-06 at 04:32 -0800, Kees Cook wrote:
> > In the past, making kallsyms entirely unreadable seemed to break
> > weird
> > stuff in userspace. How about having an alternative view that just
> > contains a alphanumeric sort of the symbol names (and they will
> > continue
> > to have zeroed addresses for unprivileged users)?
> >
> > Or perhaps we wait to hear about this causing a problem, and deal
> > with
> > it then? :)
> >
>
> Yeah - I don't know what people want here. Clearly, we can't leave
> kallsyms the way it is. Removing it entirely is a pretty fast way to
> figure out how people use it though :).
FYI, a pretty decent way to see how people are using an API is
codesearch.debian.net, which searches through the source code of all
the packages debian ships:
https://codesearch.debian.net/search?q=%2Fproc%2Fkallsyms&literal=1
On 02/06/20 at 09:51am, Kristen Carlson Accardi wrote:
> On Thu, 2020-02-06 at 04:32 -0800, Kees Cook wrote:
> > In the past, making kallsyms entirely unreadable seemed to break
> > weird
> > stuff in userspace. How about having an alternative view that just
> > contains a alphanumeric sort of the symbol names (and they will
> > continue
> > to have zeroed addresses for unprivileged users)?
> >
> > Or perhaps we wait to hear about this causing a problem, and deal
> > with
> > it then? :)
> >
>
> Yeah - I don't know what people want here. Clearly, we can't leave
> kallsyms the way it is. Removing it entirely is a pretty fast way to
> figure out how people use it though :).
Kexec-tools and makedumpfile are the users of /proc/kallsyms currently.
We use kallsyms to get page_offset_base and _stext.
On Thu, Feb 27, 2020 at 10:42:53AM +0800, Baoquan He wrote:
> On 02/06/20 at 09:51am, Kristen Carlson Accardi wrote:
> > On Thu, 2020-02-06 at 04:32 -0800, Kees Cook wrote:
>
> > > In the past, making kallsyms entirely unreadable seemed to break
> > > weird
> > > stuff in userspace. How about having an alternative view that just
> > > contains a alphanumeric sort of the symbol names (and they will
> > > continue
> > > to have zeroed addresses for unprivileged users)?
> > >
> > > Or perhaps we wait to hear about this causing a problem, and deal
> > > with
> > > it then? :)
> > >
> >
> > Yeah - I don't know what people want here. Clearly, we can't leave
> > kallsyms the way it is. Removing it entirely is a pretty fast way to
> > figure out how people use it though :).
>
> Kexec-tools and makedumpfile are the users of /proc/kallsyms currently.
> We use kallsyms to get page_offset_base and _stext.
AIUI, those run as root so they'd be able to consume the uncensored
output.
--
Kees Cook
On 02/27/20 at 08:02am, Kees Cook wrote:
> On Thu, Feb 27, 2020 at 10:42:53AM +0800, Baoquan He wrote:
> > On 02/06/20 at 09:51am, Kristen Carlson Accardi wrote:
> > > On Thu, 2020-02-06 at 04:32 -0800, Kees Cook wrote:
> >
> > > > In the past, making kallsyms entirely unreadable seemed to break
> > > > weird
> > > > stuff in userspace. How about having an alternative view that just
> > > > contains a alphanumeric sort of the symbol names (and they will
> > > > continue
> > > > to have zeroed addresses for unprivileged users)?
> > > >
> > > > Or perhaps we wait to hear about this causing a problem, and deal
> > > > with
> > > > it then? :)
> > > >
> > >
> > > Yeah - I don't know what people want here. Clearly, we can't leave
> > > kallsyms the way it is. Removing it entirely is a pretty fast way to
> > > figure out how people use it though :).
> >
> > Kexec-tools and makedumpfile are the users of /proc/kallsyms currently.
> > We use kallsyms to get page_offset_base and _stext.
>
> AIUI, those run as root so they'd be able to consume the uncensored
> output.
Yes, they have to be run under root.
On Thu, 2020-02-06 at 20:27 +0100, Jann Horn wrote:
> On Thu, Feb 6, 2020 at 6:51 PM Kristen Carlson Accardi
> <[email protected]> wrote:
> > On Thu, 2020-02-06 at 04:32 -0800, Kees Cook wrote:
> > > In the past, making kallsyms entirely unreadable seemed to break
> > > weird
> > > stuff in userspace. How about having an alternative view that
> > > just
> > > contains a alphanumeric sort of the symbol names (and they will
> > > continue
> > > to have zeroed addresses for unprivileged users)?
> > >
> > > Or perhaps we wait to hear about this causing a problem, and deal
> > > with
> > > it then? :)
> > >
> >
> > Yeah - I don't know what people want here. Clearly, we can't leave
> > kallsyms the way it is. Removing it entirely is a pretty fast way
> > to
> > figure out how people use it though :).
>
> FYI, a pretty decent way to see how people are using an API is
> codesearch.debian.net, which searches through the source code of all
> the packages debian ships:
>
> https://codesearch.debian.net/search?q=%2Fproc%2Fkallsyms&literal=1
I looked through some of these packages as Jann suggested, and it seems
like there are several that are using /proc/kallsyms to look for
specific symbol names to determine whether some feature has been
compiled into the kernel. This practice seems dubious to me, knowing
that many kernel symbol names can be changed at any time, but
regardless seems to be fairly common.
On Mon, Mar 02, 2020 at 11:01:56AM -0800, Kristen Carlson Accardi wrote:
> On Thu, 2020-02-06 at 20:27 +0100, Jann Horn wrote:
> > https://codesearch.debian.net/search?q=%2Fproc%2Fkallsyms&literal=1
>
> I looked through some of these packages as Jann suggested, and it seems
> like there are several that are using /proc/kallsyms to look for
> specific symbol names to determine whether some feature has been
> compiled into the kernel. This practice seems dubious to me, knowing
> that many kernel symbol names can be changed at any time, but
> regardless seems to be fairly common.
Cool, so a sorted censored list is fine for non-root. Would root users
break on a symbol-name-sorted view? (i.e. are two lists needed or can we
stick to one?)
--
Kees Cook
On Mon, 2020-03-02 at 11:08 -0800, Kees Cook wrote:
> On Mon, Mar 02, 2020 at 11:01:56AM -0800, Kristen Carlson Accardi
> wrote:
> > On Thu, 2020-02-06 at 20:27 +0100, Jann Horn wrote:
> > > https://codesearch.debian.net/search?q=%2Fproc%2Fkallsyms&literal=1
> >
> > I looked through some of these packages as Jann suggested, and it
> > seems
> > like there are several that are using /proc/kallsyms to look for
> > specific symbol names to determine whether some feature has been
> > compiled into the kernel. This practice seems dubious to me,
> > knowing
> > that many kernel symbol names can be changed at any time, but
> > regardless seems to be fairly common.
>
> Cool, so a sorted censored list is fine for non-root. Would root
> users
> break on a symbol-name-sorted view? (i.e. are two lists needed or can
> we
> stick to one?)
>
Internally of course we'll always have to have 2 lists. I couldn't find
any examples of even root users needing the list to be in order by
address. At the same time, it feels like a less risky thing to do to
leave root users with the same thing they've always had and only muck
with non-root users.