| Message ID | 1497915397-93805-22-git-send-email-keescook@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi David + Kees, On Mon, Jun 19, 2017 at 04:36:35PM -0700, Kees Cook wrote: > With all known usercopied cache whitelists now defined in the kernel, switch > the default usercopy region of kmem_cache_create() to size 0. Any new caches > with usercopy regions will now need to use kmem_cache_create_usercopy() > instead of kmem_cache_create(). > While I'd certainly like to see the caches be whitelisted, it needs to be made very clear that it's being done (the cover letter for this series falsely claims that kmem_cache_create() is unchanged) and what the consequences are. Is there any specific plan for identifying caches that were missed? If it's expected for people to just fix them as they are found, then they need to be helped a little --- at the very least by putting a big comment above report_usercopy() that explains the possible reasons why the error might have triggered and what to do about it. - Eric
On Mon, Jun 19, 2017 at 9:04 PM, Eric Biggers <ebiggers3@gmail.com> wrote: > Hi David + Kees, > > On Mon, Jun 19, 2017 at 04:36:35PM -0700, Kees Cook wrote: >> With all known usercopied cache whitelists now defined in the kernel, switch >> the default usercopy region of kmem_cache_create() to size 0. Any new caches >> with usercopy regions will now need to use kmem_cache_create_usercopy() >> instead of kmem_cache_create(). >> > > While I'd certainly like to see the caches be whitelisted, it needs to be made > very clear that it's being done (the cover letter for this series falsely claims > that kmem_cache_create() is unchanged) and what the consequences are. Is there Well, in the first patch it is semantically unchanged: calls to kmem_cache_create() after the first patch whitelist the entire cache object. Only from this patch on does it change behavior to no longer whitelist the object. > any specific plan for identifying caches that were missed? If it's expected for The plan for finding caches needing whitelisting is mainly code audit and operational testing. Encountering it is quite loud in that it BUGs the kernel during the hardened usercopy checks. > people to just fix them as they are found, then they need to be helped a little > --- at the very least by putting a big comment above report_usercopy() that > explains the possible reasons why the error might have triggered and what to do > about it. That sounds reasonable. It should have a comment even for the existing protections. Thanks! -Kees
diff --git a/mm/slab_common.c b/mm/slab_common.c index 685321a0d355..2365dd21623d 100644 --- a/mm/slab_common.c +++ b/mm/slab_common.c @@ -512,7 +512,7 @@ struct kmem_cache * kmem_cache_create(const char *name, size_t size, size_t align, unsigned long flags, void (*ctor)(void *)) { - return kmem_cache_create_usercopy(name, size, align, flags, 0, size, + return kmem_cache_create_usercopy(name, size, align, flags, 0, 0, ctor); } EXPORT_SYMBOL(kmem_cache_create);
With all known usercopied cache whitelists now defined in the kernel, switch the default usercopy region of kmem_cache_create() to size 0. Any new caches with usercopy regions will now need to use kmem_cache_create_usercopy() instead of kmem_cache_create(). This patch is modified from Brad Spengler/PaX Team's PAX_USERCOPY whitelisting code in the last public patch of grsecurity/PaX based on my understanding of the code. Changes or omissions from the original code are mine and don't reflect the original grsecurity/PaX code. Signed-off-by: Kees Cook <keescook@chromium.org> Cc: David Windsor <dave@nullcore.net> --- mm/slab_common.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)