From mboxrd@z Thu Jan 1 00:00:00 1970 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751948AbeAPQyf (ORCPT + 1 other); Tue, 16 Jan 2018 11:54:35 -0500 Received: from resqmta-ch2-10v.sys.comcast.net ([69.252.207.42]:57896 "EHLO resqmta-ch2-10v.sys.comcast.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751831AbeAPQya (ORCPT ); Tue, 16 Jan 2018 11:54:30 -0500 Date: Tue, 16 Jan 2018 10:54:27 -0600 (CST) From: Christopher Lameter X-X-Sender: cl@nuc-kabylake To: Matthew Wilcox cc: Kees Cook , linux-kernel@vger.kernel.org, David Windsor , Pekka Enberg , David Rientjes , Joonsoo Kim , Andrew Morton , linux-mm@kvack.org, linux-xfs@vger.kernel.org, Linus Torvalds , Alexander Viro , Andy Lutomirski , Christoph Hellwig , "David S. Miller" , Laura Abbott , Mark Rutland , "Martin K. Petersen" , Paolo Bonzini , Christian Borntraeger , Christoffer Dall , Dave Kleikamp , Jan Kara , Luis de Bethencourt , Marc Zyngier , Rik van Riel , Matthew Garrett , linux-fsdevel@vger.kernel.org, linux-arch@vger.kernel.org, netdev@vger.kernel.org, kernel-hardening@lists.openwall.com Subject: Re: kmem_cache_attr (was Re: [PATCH 04/36] usercopy: Prepare for usercopy whitelisting) In-Reply-To: <20180116160525.GF30073@bombadil.infradead.org> Message-ID: References: <1515531365-37423-1-git-send-email-keescook@chromium.org> <1515531365-37423-5-git-send-email-keescook@chromium.org> <20180114230719.GB32027@bombadil.infradead.org> <20180116160525.GF30073@bombadil.infradead.org> User-Agent: Alpine 2.20 (DEB 67 2015-01-07) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-CMAE-Envelope: MS4wfDRr79wWEQYf0lRGsiqWGjjxKpjOQE6I700lfyjQDv+oBYG2F/L4LlxdNK46mH8Mz+0s8GQRpKdlzftDwzCufobuJ/nGe8Ah14TXgRfzYHEHiT/toy07 rczIEPSqApemXUBoigDybd6pUG6LrHVx8ssApShufFoP4X2PjeQmqJ0TUXKIMAQish6Xz7w7e9Z+imAxZM4VOSKE7lh0APUQefsolZBUqwc78aF6vifNT9I+ xuzmb6Sma+sYZrWeUqKQg7atZLmCLA065GvhgyEWz16PNJ+uqRgUazGlvvgLd74G5tocmZU1kEol4gycR0oYZrlMhzzjgfhlB3/C8IS8sOqT9+l7y0ftkwTK BqTNB+t3hFxDlt+WaSgdkKzlhJ2mcvyv7YHdGkpIB/SXy6ryQGkJRtVOingnGiGIn9QMswmgUjQYiZ3Ybzq+IPKJXe4Rn/3Zs1DjsEklnjqqJdzhJzG0QNqy +R/wT6Q4dD9WlPqprTHhEeEGEwzgoLfUmH7LOY2GQVSQJiLtQm90WZTXnY+2ofjmQAe99uwI1zfIoL3jiZJqAeTj560zFBoLm68Bn3OWBEGETDV4Z4Mw30Ep 9yUIwmmDEwQGIkULy3KrHNpn1PMkgjH/eyRO3bZIKgyEfO2vvliPzEpUur8dA0Y4lcdQTgB8KZoZ41HKOzyUcmSl5NUHcUHpSm1Gbzzbyk26ns2hOlSp7wwq L/aq+kswNADOxy8Kaf//pO00sF3nln4jXNqkULMNUccQY0c2+fZkenrKrOC9nH5zjyqKuBZb2uXsa0i5VGNUhXM+3OkUuriZ5knKjZCkcqN43t4HDhBvhSEb EQtTa5fBaBYg3AkGS6RVqiA+DKzQ3l/XvrzfJNiN0tGiQM8Cs7O7BwT9UvyVDEWh/mfzI57+fgzVTrMUenE/uvM2ujtKvM0oP1YCclxf1BItaLpXQQME+VA1 NSa5M5t1w1VkisHA4P7pvSB49PdEI/GyPAzqKg3Vj2vL48sNC2q0Rbx1ZRs3jcrssxUvMS2gnsiS9/VERgCP7YxBr4the6CqoUfGb+Jm+Xbw3QkTGO4iVE/8 LRzr1dFXzMnuAyiNhU/qItuVomPEoQWnGgaL59G4BwhUFlN5i1E82zEcDE0rPQZjKIFu26TdIyNTAXRsRdDOMUpGf2stCJiza8o= Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Return-Path: On Tue, 16 Jan 2018, Matthew Wilcox wrote: > I think that's a good thing! /proc/slabinfo really starts to get grotty > above 16 bytes. I'd like to chop off "_cache" from the name of every > single slab! If ext4_allocation_context has to become ext4_alloc_ctx, > I don't think we're going to lose any valuable information. Ok so we are going to cut off at 16 charaacters? Sounds good to me. > > struct kmem_cache_attr { > > char *name; > > size_t size; > > size_t align; > > slab_flags_t flags; > > unsigned int useroffset; > > unsinged int usersize; > > void (*ctor)(void *); > > kmem_isolate_func *isolate; > > kmem_migrate_func *migrate; > > ... > > } > > In these slightly-more-security-conscious days, it's considered poor > practice to have function pointers in writable memory. That was why > I wanted to make the kmem_cache_attr const. Sure this data is never changed. It can be const. > Also, there's no need for 'size' and 'align' to be size_t. Slab should > never support allocations above 4GB in size. I'm not even keen on seeing > allocations above 64kB, but I see my laptop has six 512kB allocations (!), > three 256kB allocations and seven 128kB allocations, so I must reluctantly > concede that using an unsigned int is necessary. If I were really into > bitshaving, I might force all allocations to be a multiple of 32-bytes > in size, and then we could use 16 bits to represent an allocation between > 32 and 2MB, but I think that tips us beyond the complexity boundary. I am not married to either way of specifying the sizes. unsigned int would be fine with me. SLUB falls back to the page allocator anyways for anything above 2* PAGE_SIZE and I think we can do the same for the other allocators as well. Zeroing or initializing such a large memory chunk is much more expensive than the allocation so it does not make much sense to have that directly supported in the slab allocators. Some platforms support 64K page size and I could envision a 2M page size at some point. So I think we cannot use 16 bits there. If no one objects then I can use unsigned int there again. From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christopher Lameter Subject: Re: kmem_cache_attr (was Re: [PATCH 04/36] usercopy: Prepare for usercopy whitelisting) Date: Tue, 16 Jan 2018 10:54:27 -0600 (CST) Message-ID: References: <1515531365-37423-1-git-send-email-keescook@chromium.org> <1515531365-37423-5-git-send-email-keescook@chromium.org> <20180114230719.GB32027@bombadil.infradead.org> <20180116160525.GF30073@bombadil.infradead.org> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Cc: Kees Cook , linux-kernel@vger.kernel.org, David Windsor , Pekka Enberg , David Rientjes , Joonsoo Kim , Andrew Morton , linux-mm@kvack.org, linux-xfs@vger.kernel.org, Linus Torvalds , Alexander Viro , Andy Lutomirski , Christoph Hellwig , "David S. Miller" , Laura Abbott , Mark Rutland , "Martin K. Petersen" , Paolo Bonzini , Christian Borntraeger , Christoffer Dall , Dave Kleikamp , Jan Kara , To: Matthew Wilcox Return-path: In-Reply-To: <20180116160525.GF30073@bombadil.infradead.org> Sender: owner-linux-mm@kvack.org List-Id: netdev.vger.kernel.org On Tue, 16 Jan 2018, Matthew Wilcox wrote: > I think that's a good thing! /proc/slabinfo really starts to get grotty > above 16 bytes. I'd like to chop off "_cache" from the name of every > single slab! If ext4_allocation_context has to become ext4_alloc_ctx, > I don't think we're going to lose any valuable information. Ok so we are going to cut off at 16 charaacters? Sounds good to me. > > struct kmem_cache_attr { > > char *name; > > size_t size; > > size_t align; > > slab_flags_t flags; > > unsigned int useroffset; > > unsinged int usersize; > > void (*ctor)(void *); > > kmem_isolate_func *isolate; > > kmem_migrate_func *migrate; > > ... > > } > > In these slightly-more-security-conscious days, it's considered poor > practice to have function pointers in writable memory. That was why > I wanted to make the kmem_cache_attr const. Sure this data is never changed. It can be const. > Also, there's no need for 'size' and 'align' to be size_t. Slab should > never support allocations above 4GB in size. I'm not even keen on seeing > allocations above 64kB, but I see my laptop has six 512kB allocations (!), > three 256kB allocations and seven 128kB allocations, so I must reluctantly > concede that using an unsigned int is necessary. If I were really into > bitshaving, I might force all allocations to be a multiple of 32-bytes > in size, and then we could use 16 bits to represent an allocation between > 32 and 2MB, but I think that tips us beyond the complexity boundary. I am not married to either way of specifying the sizes. unsigned int would be fine with me. SLUB falls back to the page allocator anyways for anything above 2* PAGE_SIZE and I think we can do the same for the other allocators as well. Zeroing or initializing such a large memory chunk is much more expensive than the allocation so it does not make much sense to have that directly supported in the slab allocators. Some platforms support 64K page size and I could envision a 2M page size at some point. So I think we cannot use 16 bits there. If no one objects then I can use unsigned int there again. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Tue, 16 Jan 2018 10:54:27 -0600 (CST) From: Christopher Lameter To: Matthew Wilcox cc: Kees Cook , linux-kernel@vger.kernel.org, David Windsor , Pekka Enberg , David Rientjes , Joonsoo Kim , Andrew Morton , linux-mm@kvack.org, linux-xfs@vger.kernel.org, Linus Torvalds , Alexander Viro , Andy Lutomirski , Christoph Hellwig , "David S. Miller" , Laura Abbott , Mark Rutland , "Martin K. Petersen" , Paolo Bonzini , Christian Borntraeger , Christoffer Dall , Dave Kleikamp , Jan Kara , Luis de Bethencourt , Marc Zyngier , Rik van Riel , Matthew Garrett , linux-fsdevel@vger.kernel.org, linux-arch@vger.kernel.org, netdev@vger.kernel.org, kernel-hardening@lists.openwall.com Subject: Re: kmem_cache_attr (was Re: [PATCH 04/36] usercopy: Prepare for usercopy whitelisting) In-Reply-To: <20180116160525.GF30073@bombadil.infradead.org> Message-ID: References: <1515531365-37423-1-git-send-email-keescook@chromium.org> <1515531365-37423-5-git-send-email-keescook@chromium.org> <20180114230719.GB32027@bombadil.infradead.org> <20180116160525.GF30073@bombadil.infradead.org> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Sender: owner-linux-mm@kvack.org List-ID: On Tue, 16 Jan 2018, Matthew Wilcox wrote: > I think that's a good thing! /proc/slabinfo really starts to get grotty > above 16 bytes. I'd like to chop off "_cache" from the name of every > single slab! If ext4_allocation_context has to become ext4_alloc_ctx, > I don't think we're going to lose any valuable information. Ok so we are going to cut off at 16 charaacters? Sounds good to me. > > struct kmem_cache_attr { > > char *name; > > size_t size; > > size_t align; > > slab_flags_t flags; > > unsigned int useroffset; > > unsinged int usersize; > > void (*ctor)(void *); > > kmem_isolate_func *isolate; > > kmem_migrate_func *migrate; > > ... > > } > > In these slightly-more-security-conscious days, it's considered poor > practice to have function pointers in writable memory. That was why > I wanted to make the kmem_cache_attr const. Sure this data is never changed. It can be const. > Also, there's no need for 'size' and 'align' to be size_t. Slab should > never support allocations above 4GB in size. I'm not even keen on seeing > allocations above 64kB, but I see my laptop has six 512kB allocations (!), > three 256kB allocations and seven 128kB allocations, so I must reluctantly > concede that using an unsigned int is necessary. If I were really into > bitshaving, I might force all allocations to be a multiple of 32-bytes > in size, and then we could use 16 bits to represent an allocation between > 32 and 2MB, but I think that tips us beyond the complexity boundary. I am not married to either way of specifying the sizes. unsigned int would be fine with me. SLUB falls back to the page allocator anyways for anything above 2* PAGE_SIZE and I think we can do the same for the other allocators as well. Zeroing or initializing such a large memory chunk is much more expensive than the allocation so it does not make much sense to have that directly supported in the slab allocators. Some platforms support 64K page size and I could envision a 2M page size at some point. So I think we cannot use 16 bits there. If no one objects then I can use unsigned int there again. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christopher Lameter Subject: Re: kmem_cache_attr (was Re: [PATCH 04/36] usercopy: Prepare for usercopy whitelisting) Date: Tue, 16 Jan 2018 10:54:27 -0600 (CST) Message-ID: References: <1515531365-37423-1-git-send-email-keescook@chromium.org> <1515531365-37423-5-git-send-email-keescook@chromium.org> <20180114230719.GB32027@bombadil.infradead.org> <20180116160525.GF30073@bombadil.infradead.org> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Return-path: In-Reply-To: <20180116160525.GF30073@bombadil.infradead.org> Sender: owner-linux-mm@kvack.org To: Matthew Wilcox Cc: Kees Cook , linux-kernel@vger.kernel.org, David Windsor , Pekka Enberg , David Rientjes , Joonsoo Kim , Andrew Morton , linux-mm@kvack.org, linux-xfs@vger.kernel.org, Linus Torvalds , Alexander Viro , Andy Lutomirski , Christoph Hellwig , "David S. Miller" , Laura Abbott , Mark Rutland , "Martin K. Petersen" , Paolo Bonzini , Christian Borntraeger , Christoffer Dall , Dave Kleikamp , Jan Kara List-Id: linux-arch.vger.kernel.org On Tue, 16 Jan 2018, Matthew Wilcox wrote: > I think that's a good thing! /proc/slabinfo really starts to get grotty > above 16 bytes. I'd like to chop off "_cache" from the name of every > single slab! If ext4_allocation_context has to become ext4_alloc_ctx, > I don't think we're going to lose any valuable information. Ok so we are going to cut off at 16 charaacters? Sounds good to me. > > struct kmem_cache_attr { > > char *name; > > size_t size; > > size_t align; > > slab_flags_t flags; > > unsigned int useroffset; > > unsinged int usersize; > > void (*ctor)(void *); > > kmem_isolate_func *isolate; > > kmem_migrate_func *migrate; > > ... > > } > > In these slightly-more-security-conscious days, it's considered poor > practice to have function pointers in writable memory. That was why > I wanted to make the kmem_cache_attr const. Sure this data is never changed. It can be const. > Also, there's no need for 'size' and 'align' to be size_t. Slab should > never support allocations above 4GB in size. I'm not even keen on seeing > allocations above 64kB, but I see my laptop has six 512kB allocations (!), > three 256kB allocations and seven 128kB allocations, so I must reluctantly > concede that using an unsigned int is necessary. If I were really into > bitshaving, I might force all allocations to be a multiple of 32-bytes > in size, and then we could use 16 bits to represent an allocation between > 32 and 2MB, but I think that tips us beyond the complexity boundary. I am not married to either way of specifying the sizes. unsigned int would be fine with me. SLUB falls back to the page allocator anyways for anything above 2* PAGE_SIZE and I think we can do the same for the other allocators as well. Zeroing or initializing such a large memory chunk is much more expensive than the allocation so it does not make much sense to have that directly supported in the slab allocators. Some platforms support 64K page size and I could envision a 2M page size at some point. So I think we cannot use 16 bits there. If no one objects then I can use unsigned int there again. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Date: Tue, 16 Jan 2018 10:54:27 -0600 (CST) From: Christopher Lameter In-Reply-To: <20180116160525.GF30073@bombadil.infradead.org> Message-ID: References: <1515531365-37423-1-git-send-email-keescook@chromium.org> <1515531365-37423-5-git-send-email-keescook@chromium.org> <20180114230719.GB32027@bombadil.infradead.org> <20180116160525.GF30073@bombadil.infradead.org> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Subject: [kernel-hardening] Re: kmem_cache_attr (was Re: [PATCH 04/36] usercopy: Prepare for usercopy whitelisting) To: Matthew Wilcox Cc: Kees Cook , linux-kernel@vger.kernel.org, David Windsor , Pekka Enberg , David Rientjes , Joonsoo Kim , Andrew Morton , linux-mm@kvack.org, linux-xfs@vger.kernel.org, Linus Torvalds , Alexander Viro , Andy Lutomirski , Christoph Hellwig , "David S. Miller" , Laura Abbott , Mark Rutland , "Martin K. Petersen" , Paolo Bonzini , Christian Borntraeger , Christoffer Dall , Dave Kleikamp , Jan Kara , Luis de Bethencourt , Marc Zyngier , Rik van Riel , Matthew Garrett , linux-fsdevel@vger.kernel.org, linux-arch@vger.kernel.org, netdev@vger.kernel.org, kernel-hardening@lists.openwall.com List-ID: On Tue, 16 Jan 2018, Matthew Wilcox wrote: > I think that's a good thing! /proc/slabinfo really starts to get grotty > above 16 bytes. I'd like to chop off "_cache" from the name of every > single slab! If ext4_allocation_context has to become ext4_alloc_ctx, > I don't think we're going to lose any valuable information. Ok so we are going to cut off at 16 charaacters? Sounds good to me. > > struct kmem_cache_attr { > > char *name; > > size_t size; > > size_t align; > > slab_flags_t flags; > > unsigned int useroffset; > > unsinged int usersize; > > void (*ctor)(void *); > > kmem_isolate_func *isolate; > > kmem_migrate_func *migrate; > > ... > > } > > In these slightly-more-security-conscious days, it's considered poor > practice to have function pointers in writable memory. That was why > I wanted to make the kmem_cache_attr const. Sure this data is never changed. It can be const. > Also, there's no need for 'size' and 'align' to be size_t. Slab should > never support allocations above 4GB in size. I'm not even keen on seeing > allocations above 64kB, but I see my laptop has six 512kB allocations (!), > three 256kB allocations and seven 128kB allocations, so I must reluctantly > concede that using an unsigned int is necessary. If I were really into > bitshaving, I might force all allocations to be a multiple of 32-bytes > in size, and then we could use 16 bits to represent an allocation between > 32 and 2MB, but I think that tips us beyond the complexity boundary. I am not married to either way of specifying the sizes. unsigned int would be fine with me. SLUB falls back to the page allocator anyways for anything above 2* PAGE_SIZE and I think we can do the same for the other allocators as well. Zeroing or initializing such a large memory chunk is much more expensive than the allocation so it does not make much sense to have that directly supported in the slab allocators. Some platforms support 64K page size and I could envision a 2M page size at some point. So I think we cannot use 16 bits there. If no one objects then I can use unsigned int there again.