KMSAN reported uninitialized data being written to disk when dumping
core. As a result, several kilobytes of kmalloc memory may be written to
the core file and then read by a non-privileged user.
Reported-by: sam <[email protected]>
Signed-off-by: Alexander Potapenko <[email protected]>
---
Note: Reported-by: line is subject to change
---
fs/binfmt_elf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 13f25e241ac4..25d489bc9453 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -1733,7 +1733,7 @@ static int fill_thread_core_info(struct elf_thread_core_info *t,
(!regset->active || regset->active(t->task, regset) > 0)) {
int ret;
size_t size = regset_size(t->task, regset);
- void *data = kmalloc(size, GFP_KERNEL);
+ void *data = kzalloc(size, GFP_KERNEL);
if (unlikely(!data))
return 0;
ret = regset->get(t->task, regset,
--
2.26.1.301.g55bc3eb7cb9-goog
On Sun, Apr 19, 2020 at 12:08 PM <[email protected]> wrote:
>
> KMSAN reported uninitialized data being written to disk when dumping
> core. As a result, several kilobytes of kmalloc memory may be written to
> the core file and then read by a non-privileged user.
>
> Reported-by: sam <[email protected]>
> Signed-off-by: Alexander Potapenko <[email protected]>
Please also include the following tag:
Link: https://github.com/google/kmsan/issues/76
On Sun, 19 Apr 2020 12:08:48 +0200 [email protected] wrote:
> KMSAN reported uninitialized data being written to disk when dumping
> core. As a result, several kilobytes of kmalloc memory may be written to
> the core file and then read by a non-privileged user.
>
> ...
>
> --- a/fs/binfmt_elf.c
> +++ b/fs/binfmt_elf.c
> @@ -1733,7 +1733,7 @@ static int fill_thread_core_info(struct elf_thread_core_info *t,
> (!regset->active || regset->active(t->task, regset) > 0)) {
> int ret;
> size_t size = regset_size(t->task, regset);
> - void *data = kmalloc(size, GFP_KERNEL);
> + void *data = kzalloc(size, GFP_KERNEL);
> if (unlikely(!data))
> return 0;
> ret = regset->get(t->task, regset,
This seems to be a quite easy way of exposing quite a large amount of
kernel memory contents, so I think I'll add a cc:stable to this patch?
On Mon, Apr 20, 2020 at 03:33:52PM -0700, Andrew Morton wrote:
> On Sun, 19 Apr 2020 12:08:48 +0200 [email protected] wrote:
>
> > KMSAN reported uninitialized data being written to disk when dumping
> > core. As a result, several kilobytes of kmalloc memory may be written to
> > the core file and then read by a non-privileged user.
Ewww. That's been there for 12 years. Did something change in
regset_size() or regset->get()? Do you know what leaves the hole?
> >
> > ...
> >
> > --- a/fs/binfmt_elf.c
> > +++ b/fs/binfmt_elf.c
> > @@ -1733,7 +1733,7 @@ static int fill_thread_core_info(struct elf_thread_core_info *t,
> > (!regset->active || regset->active(t->task, regset) > 0)) {
> > int ret;
> > size_t size = regset_size(t->task, regset);
> > - void *data = kmalloc(size, GFP_KERNEL);
> > + void *data = kzalloc(size, GFP_KERNEL);
> > if (unlikely(!data))
> > return 0;
> > ret = regset->get(t->task, regset,
>
> This seems to be a quite easy way of exposing quite a large amount of
> kernel memory contents, so I think I'll add a cc:stable to this patch?
Yes please.
Acked-by: Kees Cook <[email protected]>
-Kees
--
Kees Cook
On Mon, Apr 20, 2020 at 03:41:40PM -0700, Kees Cook wrote:
> On Mon, Apr 20, 2020 at 03:33:52PM -0700, Andrew Morton wrote:
> > On Sun, 19 Apr 2020 12:08:48 +0200 [email protected] wrote:
> >
> > > KMSAN reported uninitialized data being written to disk when dumping
> > > core. As a result, several kilobytes of kmalloc memory may be written to
> > > the core file and then read by a non-privileged user.
>
> Ewww. That's been there for 12 years. Did something change in
> regset_size() or regset->get()? Do you know what leaves the hole?
Not lately and I would also like to hear the details; which regset it is?
Should be reasonably easy to find - just memset() the damn thing to something
recognizable, do whatever triggers that KMSAN report and look at that
resulting coredump.
On Tue, Apr 21, 2020 at 12:33 AM Andrew Morton
<[email protected]> wrote:
>
> On Sun, 19 Apr 2020 12:08:48 +0200 [email protected] wrote:
>
> > KMSAN reported uninitialized data being written to disk when dumping
> > core. As a result, several kilobytes of kmalloc memory may be written to
> > the core file and then read by a non-privileged user.
> >
> > ...
> >
> > --- a/fs/binfmt_elf.c
> > +++ b/fs/binfmt_elf.c
> > @@ -1733,7 +1733,7 @@ static int fill_thread_core_info(struct elf_thread_core_info *t,
> > (!regset->active || regset->active(t->task, regset) > 0)) {
> > int ret;
> > size_t size = regset_size(t->task, regset);
> > - void *data = kmalloc(size, GFP_KERNEL);
> > + void *data = kzalloc(size, GFP_KERNEL);
> > if (unlikely(!data))
> > return 0;
> > ret = regset->get(t->task, regset,
>
> This seems to be a quite easy way of exposing quite a large amount of
> kernel memory contents, so I think I'll add a cc:stable to this patch?
Correct. Sorry, I forgot about this.
The reporter is also happy with the Reported-by: tag, so you can
ignore the note.
--
Alexander Potapenko
Software Engineer
Google Germany GmbH
Erika-Mann-Straße, 33
80636 München
Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
> Ewww. That's been there for 12 years. Did something change in
> regset_size() or regset->get()? Do you know what leaves the hole?
I don't think anything changed on the kernel side recently.
But I've made some changes to DMA handling recently, which made uninit
writes to the disk discoverable.
We don't use core dumping on syzbot (because the auto-generated
programs crash all the time), so it could be a very old bug that was
triggered in a non-standard setup.
> Not lately and I would also like to hear the details; which regset it is?
> Should be reasonably easy to find - just memset() the damn thing to something
> recognizable, do whatever triggers that KMSAN report and look at that
> resulting coredump.
The bug is easily triggerable by the following program:
================================================
int main() {
volatile char *c = 0;
(void)*c;
return 0;
}
================================================
in my QEMU after I do `ulimit -c 10000`.
I use the following patch:
================================================
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index f4713ea76e827..570193a1b291d 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -1734,6 +1734,7 @@ static int fill_thread_core_info(struct
elf_thread_core_info *t,
int ret;
size_t size = regset_size(t->task, regset);
void *data = kmalloc(size, GFP_KERNEL);
+ memset(data, 0xae, size);
if (unlikely(!data))
return 0;
ret = regset->get(t->task, regset,
================================================
to find uninitialized bytes in the core file using hexdump.
Those are:
0x96c--0x984 (24 bytes)
0x98c--0xa0c (128 bytes)
0xb0c--0xb3c (48 bytes)
0xbac--0x13ec (2112 bytes)
I added kmsan_check_memory(addr, nr) to dump_emit() and got the
following reports when calling dump_emit(cprm, men->data, men->datasz)
in writenote():
BUG: KMSAN: uninit-value in kmsan_check_memory+0xd/0x10
mm/kmsan/kmsan_hooks.c:428
Bytes 0-23 of 2696 are uninitialized
BUG: KMSAN: uninit-value in kmsan_check_memory+0xd/0x10
mm/kmsan/kmsan_hooks.c:428
Bytes 32-159 of 2696 are uninitialized
BUG: KMSAN: uninit-value in kmsan_check_memory+0xd/0x10
mm/kmsan/kmsan_hooks.c:428
Bytes 416-463 of 2696 are uninitialized
BUG: KMSAN: uninit-value in kmsan_check_memory+0xd/0x10
mm/kmsan/kmsan_hooks.c:428
Bytes 576-2687 of 2696 are uninitialized
FWIW one of such reports looks as follows:
=====================================================
BUG: KMSAN: uninit-value in kmsan_check_memory+0xd/0x10
mm/kmsan/kmsan_hooks.c:428
CPU: 0 PID: 1503 Comm: probe Not tainted 5.6.0-rc7+ #4105
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1 04/01/2014
Call Trace:
__dump_stack lib/dump_stack.c:77
dump_stack+0x16f/0x208 lib/dump_stack.c:118
kmsan_report+0xf7/0x1e0 mm/kmsan/kmsan_report.c:118
kmsan_internal_check_memory+0x22e/0x3c0 mm/kmsan/kmsan.c:423
kmsan_check_memory+0xd/0x10 mm/kmsan/kmsan_hooks.c:428
dump_emit+0x1ab/0x580 fs/coredump.c:824
writenote+0x30a/0x4b0 fs/binfmt_elf.c:1420
free_note_info fs/binfmt_elf.c:1910
elf_core_dump+0x644c/0x6e60 fs/binfmt_elf.c:2377
do_coredump+0x488d/0x5400 fs/coredump.c:790
get_signal+0x147f/0x2d50 kernel/signal.c:2733
do_signal+0x6d/0xda0 arch/x86/kernel/signal.c:813
exit_to_usermode_loop arch/x86/entry/common.c:160
...
Uninit was created at:
kmsan_save_stack_with_flags mm/kmsan/kmsan.c:144
kmsan_internal_poison_shadow+0x5c/0xf0 mm/kmsan/kmsan.c:127
kmsan_poison_shadow+0x5f/0xa0 mm/kmsan/kmsan_hooks.c:405
kmalloc ./include/linux/slab.h:560
fill_thread_core_info fs/binfmt_elf.c:1736
fill_note_info fs/binfmt_elf.c:1838
elf_core_dump+0x2f8c/0x6e60 fs/binfmt_elf.c:2239
do_coredump+0x488d/0x5400 fs/coredump.c:790
get_signal+0x147f/0x2d50 kernel/signal.c:2733
do_signal+0x6d/0xda0 arch/x86/kernel/signal.c:813
exit_to_usermode_loop arch/x86/entry/common.c:160
prepare_exit_to_usermode+0x337/0x480 arch/x86/entry/common.c:195
swapgs_restore_regs_and_return_to_usermode+0x0/0x80
arch/x86/entry/entry_64.S:625
Bytes 0-23 of 2696 are uninitialized
Memory access of size 2696 starts at ffff9e85a7115000
=====================================================
--
Alexander Potapenko
Software Engineer
Google Germany GmbH
Erika-Mann-Straße, 33
80636 München
Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
On Tue, Apr 21, 2020 at 5:42 AM Al Viro <[email protected]> wrote:
>
> On Mon, Apr 20, 2020 at 03:41:40PM -0700, Kees Cook wrote:
> > On Mon, Apr 20, 2020 at 03:33:52PM -0700, Andrew Morton wrote:
> > > On Sun, 19 Apr 2020 12:08:48 +0200 [email protected] wrote:
> > >
> > > > KMSAN reported uninitialized data being written to disk when dumping
> > > > core. As a result, several kilobytes of kmalloc memory may be written to
> > > > the core file and then read by a non-privileged user.
> >
> > Ewww. That's been there for 12 years. Did something change in
> > regset_size() or regset->get()? Do you know what leaves the hole?
>
> Not lately and I would also like to hear the details; which regset it is?
> Should be reasonably easy to find - just memset() the damn thing to something
> recognizable, do whatever triggers that KMSAN report and look at that
> resulting coredump.
>
Seems to be REGSET_XSTATE filled by xstateregs_get().
Is there a ptrace interface also using that function?
--
Alexander Potapenko
Software Engineer
Google Germany GmbH
Erika-Mann-Straße, 33
80636 München
Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
+x86 folks
(rest of thread is on lore
<https://lore.kernel.org/lkml/[email protected]/>,
with original bug report on github
<https://github.com/google/kmsan/issues/76>)
On Tue, Apr 21, 2020 at 2:54 PM Alexander Potapenko <[email protected]> wrote:
> On Tue, Apr 21, 2020 at 5:42 AM Al Viro <[email protected]> wrote:
> > On Mon, Apr 20, 2020 at 03:41:40PM -0700, Kees Cook wrote:
> > > On Mon, Apr 20, 2020 at 03:33:52PM -0700, Andrew Morton wrote:
> > > > On Sun, 19 Apr 2020 12:08:48 +0200 [email protected] wrote:
> > > >
> > > > > KMSAN reported uninitialized data being written to disk when dumping
> > > > > core. As a result, several kilobytes of kmalloc memory may be written to
> > > > > the core file and then read by a non-privileged user.
> > >
> > > Ewww. That's been there for 12 years. Did something change in
> > > regset_size() or regset->get()? Do you know what leaves the hole?
> >
> > Not lately and I would also like to hear the details; which regset it is?
> > Should be reasonably easy to find - just memset() the damn thing to something
> > recognizable, do whatever triggers that KMSAN report and look at that
> > resulting coredump.
> >
>
> Seems to be REGSET_XSTATE filled by xstateregs_get().
> Is there a ptrace interface also using that function?
It looks to me like the problem KMSAN found is that
copy_xstate_to_kernel() will not fill out memory for unused xstates? I
think this may have been introduced by commit 91c3dba7dbc1
("x86/fpu/xstate: Fix PTRACE frames for XSAVES", introduced in v4.8).
There seem to be no other functions that reach that path other than
coredumping; I think the correct fix would be to change
copy_xstate_to_kernel() to always fully initialize the output buffer.
The ptrace path uses copy_xstate_to_user(); there, instead of leaking
kernel memory to userspace, parts of the userspace buffer are simply
not written to at all.
On Tue, 2020-04-21 at 17:09 +0200, Jann Horn wrote:
> +x86 folks
>
> (rest of thread is on lore
> <https://lore.kernel.org/lkml/[email protected]/>;,
> with original bug report on github
> <https://github.com/google/kmsan/issues/76>;)
>
> On Tue, Apr 21, 2020 at 2:54 PM Alexander Potapenko <[email protected]> wrote:
> > On Tue, Apr 21, 2020 at 5:42 AM Al Viro <[email protected]> wrote:
> > > On Mon, Apr 20, 2020 at 03:41:40PM -0700, Kees Cook wrote:
> > > > On Mon, Apr 20, 2020 at 03:33:52PM -0700, Andrew Morton wrote:
> > > > > On Sun, 19 Apr 2020 12:08:48 +0200 [email protected] wrote:
> > > > >
> > > > > > KMSAN reported uninitialized data being written to disk when dumping
> > > > > > core. As a result, several kilobytes of kmalloc memory may be written to
> > > > > > the core file and then read by a non-privileged user.
> > > >
> > > > Ewww. That's been there for 12 years. Did something change in
> > > > regset_size() or regset->get()? Do you know what leaves the hole?
> > >
> > > Not lately and I would also like to hear the details; which regset it is?
> > > Should be reasonably easy to find - just memset() the damn thing to something
> > > recognizable, do whatever triggers that KMSAN report and look at that
> > > resulting coredump.
> > >
> >
> > Seems to be REGSET_XSTATE filled by xstateregs_get().
> > Is there a ptrace interface also using that function?
>
> It looks to me like the problem KMSAN found is that
> copy_xstate_to_kernel() will not fill out memory for unused xstates? I
> think this may have been introduced by commit 91c3dba7dbc1
> ("x86/fpu/xstate: Fix PTRACE frames for XSAVES", introduced in v4.8).
>
> There seem to be no other functions that reach that path other than
> coredumping; I think the correct fix would be to change
> copy_xstate_to_kernel() to always fully initialize the output buffer.
Yes, that makes sense. On the other hand, the kzalloc() fix prevents potential
similar problems for other regsets.
Yu-cheng
>
> The ptrace path uses copy_xstate_to_user(); there, instead of leaking
> kernel memory to userspace, parts of the userspace buffer are simply
> not written to at all.
On Tue, Apr 21, 2020 at 6:05 PM Yu-cheng Yu <[email protected]> wrote:
> On Tue, 2020-04-21 at 17:09 +0200, Jann Horn wrote:
> > +x86 folks
> >
> > (rest of thread is on lore
> > <https://lore.kernel.org/lkml/[email protected]/>;,
> > with original bug report on github
> > <https://github.com/google/kmsan/issues/76>;)
> >
> > On Tue, Apr 21, 2020 at 2:54 PM Alexander Potapenko <[email protected]> wrote:
> > > On Tue, Apr 21, 2020 at 5:42 AM Al Viro <[email protected]> wrote:
> > > > On Mon, Apr 20, 2020 at 03:41:40PM -0700, Kees Cook wrote:
> > > > > On Mon, Apr 20, 2020 at 03:33:52PM -0700, Andrew Morton wrote:
> > > > > > On Sun, 19 Apr 2020 12:08:48 +0200 [email protected] wrote:
> > > > > >
> > > > > > > KMSAN reported uninitialized data being written to disk when dumping
> > > > > > > core. As a result, several kilobytes of kmalloc memory may be written to
> > > > > > > the core file and then read by a non-privileged user.
> > > > >
> > > > > Ewww. That's been there for 12 years. Did something change in
> > > > > regset_size() or regset->get()? Do you know what leaves the hole?
> > > >
> > > > Not lately and I would also like to hear the details; which regset it is?
> > > > Should be reasonably easy to find - just memset() the damn thing to something
> > > > recognizable, do whatever triggers that KMSAN report and look at that
> > > > resulting coredump.
> > > >
> > >
> > > Seems to be REGSET_XSTATE filled by xstateregs_get().
> > > Is there a ptrace interface also using that function?
> >
> > It looks to me like the problem KMSAN found is that
> > copy_xstate_to_kernel() will not fill out memory for unused xstates? I
> > think this may have been introduced by commit 91c3dba7dbc1
> > ("x86/fpu/xstate: Fix PTRACE frames for XSAVES", introduced in v4.8).
> >
> > There seem to be no other functions that reach that path other than
> > coredumping; I think the correct fix would be to change
> > copy_xstate_to_kernel() to always fully initialize the output buffer.
>
> Yes, that makes sense. On the other hand, the kzalloc() fix prevents potential
> similar problems for other regsets.
I don't really have anything against using kzalloc() there; but in my
opinion that's not a fix, that's hardening. The real problem, in my
opinion, is that regset->get() claims to have filled out a buffer
without actually having done so; and if someone happens to add another
caller to that thing in the future, I don't want them to run into
exactly the same problem again.
On Tue, 2020-04-21 at 18:16 +0200, Jann Horn wrote:
> On Tue, Apr 21, 2020 at 6:05 PM Yu-cheng Yu <[email protected]> wrote:
> > On Tue, 2020-04-21 at 17:09 +0200, Jann Horn wrote:
> > > +x86 folks
> > >
> > > (rest of thread is on lore
> > > <https://lore.kernel.org/lkml/[email protected]/>;;,
> > > with original bug report on github
> > > <https://github.com/google/kmsan/issues/76>;;)
> > >
> > > On Tue, Apr 21, 2020 at 2:54 PM Alexander Potapenko <[email protected]> wrote:
> > > > On Tue, Apr 21, 2020 at 5:42 AM Al Viro <[email protected]> wrote:
> > > > > On Mon, Apr 20, 2020 at 03:41:40PM -0700, Kees Cook wrote:
> > > > > > On Mon, Apr 20, 2020 at 03:33:52PM -0700, Andrew Morton wrote:
> > > > > > > On Sun, 19 Apr 2020 12:08:48 +0200 [email protected] wrote:
> > > > > > >
> > > > > > > > KMSAN reported uninitialized data being written to disk when dumping
> > > > > > > > core. As a result, several kilobytes of kmalloc memory may be written to
> > > > > > > > the core file and then read by a non-privileged user.
> > > > > >
> > > > > > Ewww. That's been there for 12 years. Did something change in
> > > > > > regset_size() or regset->get()? Do you know what leaves the hole?
> > > > >
> > > > > Not lately and I would also like to hear the details; which regset it is?
> > > > > Should be reasonably easy to find - just memset() the damn thing to something
> > > > > recognizable, do whatever triggers that KMSAN report and look at that
> > > > > resulting coredump.
> > > > >
> > > >
> > > > Seems to be REGSET_XSTATE filled by xstateregs_get().
> > > > Is there a ptrace interface also using that function?
> > >
> > > It looks to me like the problem KMSAN found is that
> > > copy_xstate_to_kernel() will not fill out memory for unused xstates? I
> > > think this may have been introduced by commit 91c3dba7dbc1
> > > ("x86/fpu/xstate: Fix PTRACE frames for XSAVES", introduced in v4.8).
> > >
> > > There seem to be no other functions that reach that path other than
> > > coredumping; I think the correct fix would be to change
> > > copy_xstate_to_kernel() to always fully initialize the output buffer.
> >
> > Yes, that makes sense. On the other hand, the kzalloc() fix prevents potential
> > similar problems for other regsets.
>
> I don't really have anything against using kzalloc() there; but in my
> opinion that's not a fix, that's hardening. The real problem, in my
> opinion, is that regset->get() claims to have filled out a buffer
> without actually having done so; and if someone happens to add another
> caller to that thing in the future, I don't want them to run into
> exactly the same problem again.
Agree!
On Tue, Apr 21, 2020 at 06:16:25PM +0200, Jann Horn wrote:
> On Tue, Apr 21, 2020 at 6:05 PM Yu-cheng Yu <[email protected]> wrote:
> > On Tue, 2020-04-21 at 17:09 +0200, Jann Horn wrote:
> > > +x86 folks
> > >
> > > (rest of thread is on lore
> > > <https://lore.kernel.org/lkml/[email protected]/>;,
> > > with original bug report on github
> > > <https://github.com/google/kmsan/issues/76>;)
> > >
> > > On Tue, Apr 21, 2020 at 2:54 PM Alexander Potapenko <[email protected]> wrote:
> > > > On Tue, Apr 21, 2020 at 5:42 AM Al Viro <[email protected]> wrote:
> > > > > On Mon, Apr 20, 2020 at 03:41:40PM -0700, Kees Cook wrote:
> > > > > > On Mon, Apr 20, 2020 at 03:33:52PM -0700, Andrew Morton wrote:
> > > > > > > On Sun, 19 Apr 2020 12:08:48 +0200 [email protected] wrote:
> > > > > > >
> > > > > > > > KMSAN reported uninitialized data being written to disk when dumping
> > > > > > > > core. As a result, several kilobytes of kmalloc memory may be written to
> > > > > > > > the core file and then read by a non-privileged user.
> > > > > >
> > > > > > Ewww. That's been there for 12 years. Did something change in
> > > > > > regset_size() or regset->get()? Do you know what leaves the hole?
> > > > >
> > > > > Not lately and I would also like to hear the details; which regset it is?
> > > > > Should be reasonably easy to find - just memset() the damn thing to something
> > > > > recognizable, do whatever triggers that KMSAN report and look at that
> > > > > resulting coredump.
> > > > >
> > > >
> > > > Seems to be REGSET_XSTATE filled by xstateregs_get().
> > > > Is there a ptrace interface also using that function?
> > >
> > > It looks to me like the problem KMSAN found is that
> > > copy_xstate_to_kernel() will not fill out memory for unused xstates? I
> > > think this may have been introduced by commit 91c3dba7dbc1
> > > ("x86/fpu/xstate: Fix PTRACE frames for XSAVES", introduced in v4.8).
> > >
> > > There seem to be no other functions that reach that path other than
> > > coredumping; I think the correct fix would be to change
> > > copy_xstate_to_kernel() to always fully initialize the output buffer.
> >
> > Yes, that makes sense. On the other hand, the kzalloc() fix prevents potential
> > similar problems for other regsets.
>
> I don't really have anything against using kzalloc() there; but in my
> opinion that's not a fix, that's hardening. The real problem, in my
> opinion, is that regset->get() claims to have filled out a buffer
> without actually having done so; and if someone happens to add another
> caller to that thing in the future, I don't want them to run into
> exactly the same problem again.
Right -- we should fix both.
--
Kees Cook
On Tue, Apr 21, 2020 at 10:14:25AM +0200, Alexander Potapenko wrote:
> > Not lately and I would also like to hear the details; which regset it is?
> > Should be reasonably easy to find - just memset() the damn thing to something
> > recognizable, do whatever triggers that KMSAN report and look at that
> > resulting coredump.
>
> The bug is easily triggerable by the following program:
>
> ================================================
> int main() {
> volatile char *c = 0;
> (void)*c;
> return 0;
> }
> ================================================
>
> in my QEMU after I do `ulimit -c 10000`.
.config, please - I hadn't been able to reproduce that on mine.
Coredump obviously does happen, but not a trace of the poison
is there - with your memset(data, 0xae, size) added, that is.
On Tue, May 12, 2020 at 02:09:01AM +0100, Al Viro wrote:
> On Tue, Apr 21, 2020 at 10:14:25AM +0200, Alexander Potapenko wrote:
> > > Not lately and I would also like to hear the details; which regset it is?
> > > Should be reasonably easy to find - just memset() the damn thing to something
> > > recognizable, do whatever triggers that KMSAN report and look at that
> > > resulting coredump.
> >
> > The bug is easily triggerable by the following program:
> >
> > ================================================
> > int main() {
> > volatile char *c = 0;
> > (void)*c;
> > return 0;
> > }
> > ================================================
> >
> > in my QEMU after I do `ulimit -c 10000`.
>
> .config, please - I hadn't been able to reproduce that on mine.
> Coredump obviously does happen, but not a trace of the poison
> is there - with your memset(data, 0xae, size) added, that is.
Actually, more interesting question would be your /proc/cpuinfo...
On Tue, May 12, 2020 at 5:44 AM Al Viro <[email protected]> wrote:
>
> On Tue, May 12, 2020 at 02:09:01AM +0100, Al Viro wrote:
> > On Tue, Apr 21, 2020 at 10:14:25AM +0200, Alexander Potapenko wrote:
> > > > Not lately and I would also like to hear the details; which regset it is?
> > > > Should be reasonably easy to find - just memset() the damn thing to something
> > > > recognizable, do whatever triggers that KMSAN report and look at that
> > > > resulting coredump.
> > >
> > > The bug is easily triggerable by the following program:
> > >
> > > ================================================
> > > int main() {
> > > volatile char *c = 0;
> > > (void)*c;
> > > return 0;
> > > }
> > > ================================================
> > >
> > > in my QEMU after I do `ulimit -c 10000`.
> >
> > .config, please - I hadn't been able to reproduce that on mine.
> > Coredump obviously does happen, but not a trace of the poison
> > is there - with your memset(data, 0xae, size) added, that is.
>
> Actually, more interesting question would be your /proc/cpuinfo...
See both attached.
I was also able to reproduce the bug on my desktop using the attached
dump.sh script.
--
Alexander Potapenko
Software Engineer
Google Germany GmbH
Erika-Mann-Straße, 33
80636 München
Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
On Tue, May 12, 2020 at 10:20:21AM +0200, Alexander Potapenko wrote:
> On Tue, May 12, 2020 at 5:44 AM Al Viro <[email protected]> wrote:
> >
> > On Tue, May 12, 2020 at 02:09:01AM +0100, Al Viro wrote:
> > > On Tue, Apr 21, 2020 at 10:14:25AM +0200, Alexander Potapenko wrote:
> > > > > Not lately and I would also like to hear the details; which regset it is?
> > > > > Should be reasonably easy to find - just memset() the damn thing to something
> > > > > recognizable, do whatever triggers that KMSAN report and look at that
> > > > > resulting coredump.
> > > >
> > > > The bug is easily triggerable by the following program:
> > > >
> > > > ================================================
> > > > int main() {
> > > > volatile char *c = 0;
> > > > (void)*c;
> > > > return 0;
> > > > }
> > > > ================================================
> > > >
> > > > in my QEMU after I do `ulimit -c 10000`.
> > >
> > > .config, please - I hadn't been able to reproduce that on mine.
> > > Coredump obviously does happen, but not a trace of the poison
> > > is there - with your memset(data, 0xae, size) added, that is.
> >
> > Actually, more interesting question would be your /proc/cpuinfo...
>
> See both attached.
> I was also able to reproduce the bug on my desktop using the attached
> dump.sh script.
xsaves is the critical part here. FWIW, the breakage first appeared in
commit 91c3dba7dbc199191272f4a9863f86ea3bfd679f
Author: Yu-cheng Yu <[email protected]>
Date: Fri Jun 17 13:07:17 2016 -0700
x86/fpu/xstate: Fix PTRACE frames for XSAVES
XSAVES uses compacted format and is a kernel instruction. The kernel
should use standard-format, non-supervisor state data for PTRACE.
The b0rken part is
+ for (i = 0; i < XFEATURE_MAX; i++) {
+ /*
+ * Copy only in-use xstates:
+ */
+ if ((header.xfeatures >> i) & 1) {
+ void *src = __raw_xsave_addr(xsave, 1 << i);
+
+ offset = xstate_offsets[i];
+ size = xstate_sizes[i];
+
+ ret = xstate_copyout(offset, size, kbuf, ubuf, src, 0, count);
+
+ if (ret)
+ return ret;
+
+ if (offset + size >= count)
+ break;
+ }
+
+ }
The skipped parts are left uninitialized. I'm not sure what's the best
way to deal with that. Sure, we can zero the buffer passed to ->get().
However, most of the instances (and I'd looked through quite a few)
do _not_ leave uninitialized chunks. So I would rather have
xstateregs_get() zero the gaps explicitly. I'll try to put together
a sane fix when I get some sleep.
FWIW, what I'm going to do is
* make all callers of copy_regset_to_user() pass 0 as pos
(there are very few exceptions - one on arm64, three on sparc32
and five on sparc64; I hadn't dealt with arm64 one yet, but all
cases on sparc are handled)
* switch copy_regset_to_user() to doing all copyout at
once - allocate a buffer, pass it to ->get(), then copy_to_user()
the entire thing, same as coredump does
* introduce
struct membuf {
void *p;
size_t left;
};
static inline int membuf_zero(struct membuf *s, size_t size)
static inline void membuf_align(struct membuf *s, int n)
static inline int membuf_write(struct membuf *s, const void *v, size_t size)
and membuf_store(s, v) (basically, write the value of v to the damn thing,
with sizeof(v) for size).
* introduce
typedef int user_regset_get2_fn(struct task_struct *target,
const struct user_regset *regset,
struct membuf to);
and
user_regset_get2_fn *get2;
in user_regset, replacing ->get(). Instances would be using the
membuf_...() primitives for actual copying.
* convert the instances. I've done that for several architectures,
and it's _much_ cleaner than the current mess with ->get().
* get rid of user_regset_copyout() et.al. once there's no
callers left.
This bug clearly needs to be fixed in a way that would be easy
to backport, so it has go in front of that queue. I'll try to
come up with a clean fix and post it (hopefully tomorrow)...
On Wed, May 13, 2020 at 04:33:49AM +0100, Al Viro wrote:
> FWIW, what I'm going to do is
> * make all callers of copy_regset_to_user() pass 0 as pos
> (there are very few exceptions - one on arm64, three on sparc32
> and five on sparc64; I hadn't dealt with arm64 one yet, but all
> cases on sparc are handled)
[snip]
Any of that would be easy to backport, though. Several questions
regaring XSAVE and friends:
* do we ever run on XSAVE/XSAVES-capable hardware with XFEATURE_FP
turned off?
* is it possible for x86 to have gaps between the state components
area as reported by CPUID 0x0d? IOW, can area for feature 2
(XFEATURE_YMM) to start *not* at 0x200 and can area for N start
not right after the end of area for N-1 for some N > 2?
I think I have an easy-to-backport solution, but I'm really confused
about XFEATURE_FP situation...
On Mon, May 25, 2020 at 12:45:35AM +0100, Al Viro wrote:
> On Wed, May 13, 2020 at 04:33:49AM +0100, Al Viro wrote:
>
> > FWIW, what I'm going to do is
> > * make all callers of copy_regset_to_user() pass 0 as pos
> > (there are very few exceptions - one on arm64, three on sparc32
> > and five on sparc64; I hadn't dealt with arm64 one yet, but all
> > cases on sparc are handled)
>
> [snip]
>
> Any of that would be easy to backport, though. Several questions
> regaring XSAVE and friends:
>
> * do we ever run on XSAVE/XSAVES-capable hardware with XFEATURE_FP
> turned off?
>
> * is it possible for x86 to have gaps between the state components
> area as reported by CPUID 0x0d? IOW, can area for feature 2
> (XFEATURE_YMM) to start *not* at 0x200 and can area for N start
> not right after the end of area for N-1 for some N > 2?
>
> I think I have an easy-to-backport solution, but I'm really confused
> about XFEATURE_FP situation...
Folks, could you test the following?
copy_xstate_to_kernel(): don't leave parts of destination uninitialized
copy the corresponding pieces of init_fpstate into the gaps instead.
Signed-off-by: Al Viro <[email protected]>
---
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 32b153d38748..6a54e83d5589 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -957,18 +957,31 @@ static inline bool xfeatures_mxcsr_quirk(u64 xfeatures)
return true;
}
-/*
- * This is similar to user_regset_copyout(), but will not add offset to
- * the source data pointer or increment pos, count, kbuf, and ubuf.
- */
-static inline void
-__copy_xstate_to_kernel(void *kbuf, const void *data,
- unsigned int offset, unsigned int size, unsigned int size_total)
+static void fill_gap(unsigned to, void **kbuf, unsigned *pos, unsigned *count)
{
- if (offset < size_total) {
- unsigned int copy = min(size, size_total - offset);
+ if (*pos < to) {
+ unsigned size = to - *pos;
+
+ if (size > *count)
+ size = *count;
+ memcpy(*kbuf, (void *)&init_fpstate.xsave + *pos, size);
+ *kbuf += size;
+ *pos += size;
+ *count -= size;
+ }
+}
- memcpy(kbuf + offset, data, copy);
+static void copy_part(unsigned offset, unsigned size, void *from,
+ void **kbuf, unsigned *pos, unsigned *count)
+{
+ fill_gap(offset, kbuf, pos, count);
+ if (size > *count)
+ size = *count;
+ if (size) {
+ memcpy(*kbuf, from, size);
+ *kbuf += size;
+ *pos += size;
+ *count -= size;
}
}
@@ -981,8 +994,9 @@ __copy_xstate_to_kernel(void *kbuf, const void *data,
*/
int copy_xstate_to_kernel(void *kbuf, struct xregs_state *xsave, unsigned int offset_start, unsigned int size_total)
{
- unsigned int offset, size;
struct xstate_header header;
+ const unsigned off_mxcsr = offsetof(struct fxregs_state, mxcsr);
+ unsigned count = size_total;
int i;
/*
@@ -998,46 +1012,42 @@ int copy_xstate_to_kernel(void *kbuf, struct xregs_state *xsave, unsigned int of
header.xfeatures = xsave->header.xfeatures;
header.xfeatures &= ~XFEATURE_MASK_SUPERVISOR;
+ if (header.xfeatures & XFEATURE_MASK_FP)
+ copy_part(0, off_mxcsr,
+ &xsave->i387, &kbuf, &offset_start, &count);
+ if (header.xfeatures & (XFEATURE_MASK_SSE | XFEATURE_MASK_YMM))
+ copy_part(off_mxcsr, MXCSR_AND_FLAGS_SIZE,
+ &xsave->i387.mxcsr, &kbuf, &offset_start, &count);
+ if (header.xfeatures & XFEATURE_MASK_FP)
+ copy_part(offsetof(struct fxregs_state, st_space), 128,
+ &xsave->i387.st_space, &kbuf, &offset_start, &count);
+ if (header.xfeatures & XFEATURE_MASK_SSE)
+ copy_part(xstate_offsets[XFEATURE_MASK_SSE], 256,
+ &xsave->i387.xmm_space, &kbuf, &offset_start, &count);
+ /*
+ * Fill xsave->i387.sw_reserved value for ptrace frame:
+ */
+ copy_part(offsetof(struct fxregs_state, sw_reserved), 48,
+ xstate_fx_sw_bytes, &kbuf, &offset_start, &count);
/*
* Copy xregs_state->header:
*/
- offset = offsetof(struct xregs_state, header);
- size = sizeof(header);
-
- __copy_xstate_to_kernel(kbuf, &header, offset, size, size_total);
+ copy_part(offsetof(struct xregs_state, header), sizeof(header),
+ &header, &kbuf, &offset_start, &count);
- for (i = 0; i < XFEATURE_MAX; i++) {
+ for (i = FIRST_EXTENDED_XFEATURE; i < XFEATURE_MAX; i++) {
/*
* Copy only in-use xstates:
*/
if ((header.xfeatures >> i) & 1) {
void *src = __raw_xsave_addr(xsave, i);
- offset = xstate_offsets[i];
- size = xstate_sizes[i];
-
- /* The next component has to fit fully into the output buffer: */
- if (offset + size > size_total)
- break;
-
- __copy_xstate_to_kernel(kbuf, src, offset, size, size_total);
+ copy_part(xstate_offsets[i], xstate_sizes[i],
+ src, &kbuf, &offset_start, &count);
}
}
-
- if (xfeatures_mxcsr_quirk(header.xfeatures)) {
- offset = offsetof(struct fxregs_state, mxcsr);
- size = MXCSR_AND_FLAGS_SIZE;
- __copy_xstate_to_kernel(kbuf, &xsave->i387.mxcsr, offset, size, size_total);
- }
-
- /*
- * Fill xsave->i387.sw_reserved value for ptrace frame:
- */
- offset = offsetof(struct fxregs_state, sw_reserved);
- size = sizeof(xstate_fx_sw_bytes);
-
- __copy_xstate_to_kernel(kbuf, xstate_fx_sw_bytes, offset, size, size_total);
+ fill_gap(size_total, &kbuf, &offset_start, &count);
return 0;
}
On Wed, May 27, 2020 at 12:38 AM Al Viro <[email protected]> wrote:
>
> On Mon, May 25, 2020 at 12:45:35AM +0100, Al Viro wrote:
> > On Wed, May 13, 2020 at 04:33:49AM +0100, Al Viro wrote:
> >
> > > FWIW, what I'm going to do is
> > > * make all callers of copy_regset_to_user() pass 0 as pos
> > > (there are very few exceptions - one on arm64, three on sparc32
> > > and five on sparc64; I hadn't dealt with arm64 one yet, but all
> > > cases on sparc are handled)
> >
> > [snip]
> >
> > Any of that would be easy to backport, though. Several questions
> > regaring XSAVE and friends:
> >
> > * do we ever run on XSAVE/XSAVES-capable hardware with XFEATURE_FP
> > turned off?
> >
> > * is it possible for x86 to have gaps between the state components
> > area as reported by CPUID 0x0d? IOW, can area for feature 2
> > (XFEATURE_YMM) to start *not* at 0x200 and can area for N start
> > not right after the end of area for N-1 for some N > 2?
> >
> > I think I have an easy-to-backport solution, but I'm really confused
> > about XFEATURE_FP situation...
>
> Folks, could you test the following?
>
> copy_xstate_to_kernel(): don't leave parts of destination uninitialized
>
> copy the corresponding pieces of init_fpstate into the gaps instead.
>
> Signed-off-by: Al Viro <[email protected]>
Tested-by: Alexander Potapenko <[email protected]>
> ---
> diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
> index 32b153d38748..6a54e83d5589 100644
> --- a/arch/x86/kernel/fpu/xstate.c
> +++ b/arch/x86/kernel/fpu/xstate.c
> @@ -957,18 +957,31 @@ static inline bool xfeatures_mxcsr_quirk(u64 xfeatures)
> return true;
> }
>
> -/*
> - * This is similar to user_regset_copyout(), but will not add offset to
> - * the source data pointer or increment pos, count, kbuf, and ubuf.
> - */
> -static inline void
> -__copy_xstate_to_kernel(void *kbuf, const void *data,
> - unsigned int offset, unsigned int size, unsigned int size_total)
> +static void fill_gap(unsigned to, void **kbuf, unsigned *pos, unsigned *count)
> {
> - if (offset < size_total) {
> - unsigned int copy = min(size, size_total - offset);
> + if (*pos < to) {
> + unsigned size = to - *pos;
> +
> + if (size > *count)
> + size = *count;
> + memcpy(*kbuf, (void *)&init_fpstate.xsave + *pos, size);
> + *kbuf += size;
> + *pos += size;
> + *count -= size;
> + }
> +}
>
> - memcpy(kbuf + offset, data, copy);
> +static void copy_part(unsigned offset, unsigned size, void *from,
> + void **kbuf, unsigned *pos, unsigned *count)
> +{
> + fill_gap(offset, kbuf, pos, count);
> + if (size > *count)
> + size = *count;
> + if (size) {
> + memcpy(*kbuf, from, size);
> + *kbuf += size;
> + *pos += size;
> + *count -= size;
> }
> }
>
> @@ -981,8 +994,9 @@ __copy_xstate_to_kernel(void *kbuf, const void *data,
> */
> int copy_xstate_to_kernel(void *kbuf, struct xregs_state *xsave, unsigned int offset_start, unsigned int size_total)
> {
> - unsigned int offset, size;
> struct xstate_header header;
> + const unsigned off_mxcsr = offsetof(struct fxregs_state, mxcsr);
> + unsigned count = size_total;
> int i;
>
> /*
> @@ -998,46 +1012,42 @@ int copy_xstate_to_kernel(void *kbuf, struct xregs_state *xsave, unsigned int of
> header.xfeatures = xsave->header.xfeatures;
> header.xfeatures &= ~XFEATURE_MASK_SUPERVISOR;
>
> + if (header.xfeatures & XFEATURE_MASK_FP)
> + copy_part(0, off_mxcsr,
> + &xsave->i387, &kbuf, &offset_start, &count);
> + if (header.xfeatures & (XFEATURE_MASK_SSE | XFEATURE_MASK_YMM))
> + copy_part(off_mxcsr, MXCSR_AND_FLAGS_SIZE,
> + &xsave->i387.mxcsr, &kbuf, &offset_start, &count);
> + if (header.xfeatures & XFEATURE_MASK_FP)
> + copy_part(offsetof(struct fxregs_state, st_space), 128,
> + &xsave->i387.st_space, &kbuf, &offset_start, &count);
> + if (header.xfeatures & XFEATURE_MASK_SSE)
> + copy_part(xstate_offsets[XFEATURE_MASK_SSE], 256,
> + &xsave->i387.xmm_space, &kbuf, &offset_start, &count);
> + /*
> + * Fill xsave->i387.sw_reserved value for ptrace frame:
> + */
> + copy_part(offsetof(struct fxregs_state, sw_reserved), 48,
> + xstate_fx_sw_bytes, &kbuf, &offset_start, &count);
> /*
> * Copy xregs_state->header:
> */
> - offset = offsetof(struct xregs_state, header);
> - size = sizeof(header);
> -
> - __copy_xstate_to_kernel(kbuf, &header, offset, size, size_total);
> + copy_part(offsetof(struct xregs_state, header), sizeof(header),
> + &header, &kbuf, &offset_start, &count);
>
> - for (i = 0; i < XFEATURE_MAX; i++) {
> + for (i = FIRST_EXTENDED_XFEATURE; i < XFEATURE_MAX; i++) {
> /*
> * Copy only in-use xstates:
> */
> if ((header.xfeatures >> i) & 1) {
> void *src = __raw_xsave_addr(xsave, i);
>
> - offset = xstate_offsets[i];
> - size = xstate_sizes[i];
> -
> - /* The next component has to fit fully into the output buffer: */
> - if (offset + size > size_total)
> - break;
> -
> - __copy_xstate_to_kernel(kbuf, src, offset, size, size_total);
> + copy_part(xstate_offsets[i], xstate_sizes[i],
> + src, &kbuf, &offset_start, &count);
> }
>
> }
> -
> - if (xfeatures_mxcsr_quirk(header.xfeatures)) {
> - offset = offsetof(struct fxregs_state, mxcsr);
> - size = MXCSR_AND_FLAGS_SIZE;
> - __copy_xstate_to_kernel(kbuf, &xsave->i387.mxcsr, offset, size, size_total);
> - }
> -
> - /*
> - * Fill xsave->i387.sw_reserved value for ptrace frame:
> - */
> - offset = offsetof(struct fxregs_state, sw_reserved);
> - size = sizeof(xstate_fx_sw_bytes);
> -
> - __copy_xstate_to_kernel(kbuf, xstate_fx_sw_bytes, offset, size, size_total);
> + fill_gap(size_total, &kbuf, &offset_start, &count);
>
> return 0;
> }
--
Alexander Potapenko
Software Engineer
Google Germany GmbH
Erika-Mann-Straße, 33
80636 München
Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
On Wed, May 27, 2020 at 09:04:56PM +0200, Borislav Petkov wrote:
> On Tue, May 26, 2020 at 11:38:17PM +0100, Al Viro wrote:
> > Folks, could you test the following?
> >
> > copy_xstate_to_kernel(): don't leave parts of destination uninitialized
> >
> > copy the corresponding pieces of init_fpstate into the gaps instead.
> >
> > Signed-off-by: Al Viro <[email protected]>
>
> Am I taking this through tip (would prefer to as there's other FPU stuff
> pending) or should I ack this so that you can send it upwards?
Either way would work - I was going to send it to Linus tonight and an
extra Acked-by: would be welcome. OTOH, if you would rather have all
x86-related patches go through x86 git... your subtree, your rules.
On Wed, May 27, 2020 at 08:53:03PM +0100, Al Viro wrote:
> Either way would work - I was going to send it to Linus tonight and an
> extra Acked-by: would be welcome. OTOH, if you would rather have all
> x86-related patches go through x86 git... your subtree, your rules.
Ok, here we go:
Acked-by: Borislav Petkov <[email protected]>
Thx!
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On Tue, May 26, 2020 at 11:38:17PM +0100, Al Viro wrote:
> Folks, could you test the following?
>
> copy_xstate_to_kernel(): don't leave parts of destination uninitialized
>
> copy the corresponding pieces of init_fpstate into the gaps instead.
>
> Signed-off-by: Al Viro <[email protected]>
Am I taking this through tip (would prefer to as there's other FPU stuff
pending) or should I ack this so that you can send it upwards?
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On Mon, Apr 20, 2020 at 03:41:40PM -0700, Kees Cook wrote:
> On Mon, Apr 20, 2020 at 03:33:52PM -0700, Andrew Morton wrote:
> > On Sun, 19 Apr 2020 12:08:48 +0200 [email protected] wrote:
> >
> > > KMSAN reported uninitialized data being written to disk when dumping
> > > core. As a result, several kilobytes of kmalloc memory may be written to
> > > the core file and then read by a non-privileged user.
>
> Ewww. That's been there for 12 years. Did something change in
> regset_size() or regset->get()? Do you know what leaves the hole?
>
> > >
> > > ...
> > >
> > > --- a/fs/binfmt_elf.c
> > > +++ b/fs/binfmt_elf.c
> > > @@ -1733,7 +1733,7 @@ static int fill_thread_core_info(struct elf_thread_core_info *t,
> > > (!regset->active || regset->active(t->task, regset) > 0)) {
> > > int ret;
> > > size_t size = regset_size(t->task, regset);
> > > - void *data = kmalloc(size, GFP_KERNEL);
> > > + void *data = kzalloc(size, GFP_KERNEL);
> > > if (unlikely(!data))
> > > return 0;
> > > ret = regset->get(t->task, regset,
> >
> > This seems to be a quite easy way of exposing quite a large amount of
> > kernel memory contents, so I think I'll add a cc:stable to this patch?
>
> Yes please.
>
> Acked-by: Kees Cook <[email protected]>
This has been in -next for a while, but we need to get this landed and
into -stable. Can you please send this to Linus for the final release? I
know Al is working on getting the complementary fixes landed too, but
this fix is also sufficient, trivial to backport, and provides some
future-proofing/defense in depth.
-Kees
--
Kees Cook