| Message ID | 20200205223950.1212394-10-kristen@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Finer grained kernel address space randomization | expand |
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 <kristen@linux.intel.com> > --- > 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? :)
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 <kristen@linux.intel.com> > > --- > > 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 <kristen@linux.intel.com> 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.
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 > <kristen@linux.intel.com> 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?)
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.
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);
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 <kristen@linux.intel.com> --- kernel/kallsyms.c | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-)