Re: [PATCH] slab: implement kmalloc guard
On Mon, 15 Sep 2014, Joonsoo Kim wrote: > On Thu, Sep 11, 2014 at 10:32:52PM -0400, Mikulas Patocka wrote: > > > > > > On Mon, 8 Sep 2014, Christoph Lameter wrote: > > > > > On Mon, 8 Sep 2014, Mikulas Patocka wrote: > > > > > > > I don't know what you mean. If someone allocates 1 objects with > > > > sizes > > > > from 1 to 1, you can't have 1 slab caches - you can't have a > > > > slab > > > > cache for each used size. Also - you can't create a slab cache in > > > > interrupt context. > > > > > > Oh you can create them up front on bootup. And I think only the small > > > sizes matter. Allocations >=8K are pushed to the page allocator anyways. > > > > Only for SLUB. For SLAB, large allocations are still use SLAB caches up to > > 4M. But anyway - having 8K preallocated slab caches is too much. > > > > If you want to integrate this patch into the slab/slub subsystem, a better > > solution would be to store the exact size requested with kmalloc along the > > slab/slub object itself (before the preceding redzone). But it would > > result in duplicating the work - you'd have to repeat the logic in this > > patch three times - once for slab, once for slub and once for > > kmalloc_large/kmalloc_large_node. > > > > I don't know if it would be better than this patch. > > Hello, > > Out of bound write could be detected by kernel address asanitizer(KASan). > See following link. > > https://lkml.org/lkml/2014/9/10/441 > > Although this patch also looks good to me, I think that KASan is > better than this, because it could detect out of bound write and > has more features for debugging. > > Thanks. Surely, KAsan detects more bugs. But it has also high overhead. The overhead of kmalloc guard is very low. The kmalloc guard already helped to find one previously unknown bug: http://lkml.iu.edu/hypermail/linux/kernel/1409.1/02325.html Mikulas -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: implement kmalloc guard
On Mon, 15 Sep 2014, Joonsoo Kim wrote: On Thu, Sep 11, 2014 at 10:32:52PM -0400, Mikulas Patocka wrote: On Mon, 8 Sep 2014, Christoph Lameter wrote: On Mon, 8 Sep 2014, Mikulas Patocka wrote: I don't know what you mean. If someone allocates 1 objects with sizes from 1 to 1, you can't have 1 slab caches - you can't have a slab cache for each used size. Also - you can't create a slab cache in interrupt context. Oh you can create them up front on bootup. And I think only the small sizes matter. Allocations =8K are pushed to the page allocator anyways. Only for SLUB. For SLAB, large allocations are still use SLAB caches up to 4M. But anyway - having 8K preallocated slab caches is too much. If you want to integrate this patch into the slab/slub subsystem, a better solution would be to store the exact size requested with kmalloc along the slab/slub object itself (before the preceding redzone). But it would result in duplicating the work - you'd have to repeat the logic in this patch three times - once for slab, once for slub and once for kmalloc_large/kmalloc_large_node. I don't know if it would be better than this patch. Hello, Out of bound write could be detected by kernel address asanitizer(KASan). See following link. https://lkml.org/lkml/2014/9/10/441 Although this patch also looks good to me, I think that KASan is better than this, because it could detect out of bound write and has more features for debugging. Thanks. Surely, KAsan detects more bugs. But it has also high overhead. The overhead of kmalloc guard is very low. The kmalloc guard already helped to find one previously unknown bug: http://lkml.iu.edu/hypermail/linux/kernel/1409.1/02325.html Mikulas -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: implement kmalloc guard
On Thu, Sep 11, 2014 at 10:32:52PM -0400, Mikulas Patocka wrote: > > > On Mon, 8 Sep 2014, Christoph Lameter wrote: > > > On Mon, 8 Sep 2014, Mikulas Patocka wrote: > > > > > I don't know what you mean. If someone allocates 1 objects with sizes > > > from 1 to 1, you can't have 1 slab caches - you can't have a slab > > > cache for each used size. Also - you can't create a slab cache in > > > interrupt context. > > > > Oh you can create them up front on bootup. And I think only the small > > sizes matter. Allocations >=8K are pushed to the page allocator anyways. > > Only for SLUB. For SLAB, large allocations are still use SLAB caches up to > 4M. But anyway - having 8K preallocated slab caches is too much. > > If you want to integrate this patch into the slab/slub subsystem, a better > solution would be to store the exact size requested with kmalloc along the > slab/slub object itself (before the preceding redzone). But it would > result in duplicating the work - you'd have to repeat the logic in this > patch three times - once for slab, once for slub and once for > kmalloc_large/kmalloc_large_node. > > I don't know if it would be better than this patch. Hello, Out of bound write could be detected by kernel address asanitizer(KASan). See following link. https://lkml.org/lkml/2014/9/10/441 Although this patch also looks good to me, I think that KASan is better than this, because it could detect out of bound write and has more features for debugging. Thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: implement kmalloc guard
On Thu, Sep 11, 2014 at 10:32:52PM -0400, Mikulas Patocka wrote: On Mon, 8 Sep 2014, Christoph Lameter wrote: On Mon, 8 Sep 2014, Mikulas Patocka wrote: I don't know what you mean. If someone allocates 1 objects with sizes from 1 to 1, you can't have 1 slab caches - you can't have a slab cache for each used size. Also - you can't create a slab cache in interrupt context. Oh you can create them up front on bootup. And I think only the small sizes matter. Allocations =8K are pushed to the page allocator anyways. Only for SLUB. For SLAB, large allocations are still use SLAB caches up to 4M. But anyway - having 8K preallocated slab caches is too much. If you want to integrate this patch into the slab/slub subsystem, a better solution would be to store the exact size requested with kmalloc along the slab/slub object itself (before the preceding redzone). But it would result in duplicating the work - you'd have to repeat the logic in this patch three times - once for slab, once for slub and once for kmalloc_large/kmalloc_large_node. I don't know if it would be better than this patch. Hello, Out of bound write could be detected by kernel address asanitizer(KASan). See following link. https://lkml.org/lkml/2014/9/10/441 Although this patch also looks good to me, I think that KASan is better than this, because it could detect out of bound write and has more features for debugging. Thanks. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: implement kmalloc guard
On Mon, 8 Sep 2014, Christoph Lameter wrote: > On Mon, 8 Sep 2014, Mikulas Patocka wrote: > > > I don't know what you mean. If someone allocates 1 objects with sizes > > from 1 to 1, you can't have 1 slab caches - you can't have a slab > > cache for each used size. Also - you can't create a slab cache in > > interrupt context. > > Oh you can create them up front on bootup. And I think only the small > sizes matter. Allocations >=8K are pushed to the page allocator anyways. Only for SLUB. For SLAB, large allocations are still use SLAB caches up to 4M. But anyway - having 8K preallocated slab caches is too much. If you want to integrate this patch into the slab/slub subsystem, a better solution would be to store the exact size requested with kmalloc along the slab/slub object itself (before the preceding redzone). But it would result in duplicating the work - you'd have to repeat the logic in this patch three times - once for slab, once for slub and once for kmalloc_large/kmalloc_large_node. I don't know if it would be better than this patch. > > > We already have a redzone structure to check for writes over the end of > > > the object. Lets use that. > > > > So, change all three slab subsystems to use that. > > SLOB has no debugging features and I think that was intentional. We are > trying to unify the debug checks etc. Some work on that would be > appreciated. I think the kmalloc creation is already in slab_common.c Mikulas -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: implement kmalloc guard
On Mon, 8 Sep 2014, Christoph Lameter wrote: On Mon, 8 Sep 2014, Mikulas Patocka wrote: I don't know what you mean. If someone allocates 1 objects with sizes from 1 to 1, you can't have 1 slab caches - you can't have a slab cache for each used size. Also - you can't create a slab cache in interrupt context. Oh you can create them up front on bootup. And I think only the small sizes matter. Allocations =8K are pushed to the page allocator anyways. Only for SLUB. For SLAB, large allocations are still use SLAB caches up to 4M. But anyway - having 8K preallocated slab caches is too much. If you want to integrate this patch into the slab/slub subsystem, a better solution would be to store the exact size requested with kmalloc along the slab/slub object itself (before the preceding redzone). But it would result in duplicating the work - you'd have to repeat the logic in this patch three times - once for slab, once for slub and once for kmalloc_large/kmalloc_large_node. I don't know if it would be better than this patch. We already have a redzone structure to check for writes over the end of the object. Lets use that. So, change all three slab subsystems to use that. SLOB has no debugging features and I think that was intentional. We are trying to unify the debug checks etc. Some work on that would be appreciated. I think the kmalloc creation is already in slab_common.c Mikulas -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: implement kmalloc guard
On Mon, 8 Sep 2014, Mikulas Patocka wrote: > I don't know what you mean. If someone allocates 1 objects with sizes > from 1 to 1, you can't have 1 slab caches - you can't have a slab > cache for each used size. Also - you can't create a slab cache in > interrupt context. Oh you can create them up front on bootup. And I think only the small sizes matter. Allocations >=8K are pushed to the page allocator anyways. > > We already have a redzone structure to check for writes over the end of > > the object. Lets use that. > > So, change all three slab subsystems to use that. SLOB has no debugging features and I think that was intentional. We are trying to unify the debug checks etc. Some work on that would be appreciated. I think the kmalloc creation is already in slab_common.c -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: implement kmalloc guard
On Mon, 8 Sep 2014, Christoph Lameter wrote: > On Fri, 5 Sep 2014, Mikulas Patocka wrote: > > > This patch adds a new option DEBUG_KMALLOC that makes it possible to > > detect writes beyond the end of space allocated with kmalloc. Normally, > > the kmalloc function rounds the size to the next power of two (there is > > exception to this rule - sizes 96 and 192). Slab debugging detects only > > writes beyond the end of the slab object, it is unable to detect writes > > beyond the end of kmallocated size that fit into the slab object. > > > > The motivation for this patch was this: There was 6 year old bug in > > dm-crypt (d49ec52ff6ddcda178fc2476a109cf1bd1fa19ed) where the driver would > > write beyond the end of kmallocated space, but the bug went undetected > > because the write would fit into the power-of-two-sized slab object. Only > > recent changes to dm-crypt made the bug show up. There is similar problem > > in the nx-crypto driver in the function nx_crypto_ctx_init - again, > > because kmalloc rounds the size up to the next power of two, this bug went > > undetected. > > The problem with the kmalloc array for debugging is inded that it is > only for powers of two for efficiency purposes. In the debug > situation we may not have the need for that efficiency. Maybe simply > creating kmalloc arrays for each size will do the trick? I don't know what you mean. If someone allocates 1 objects with sizes from 1 to 1, you can't have 1 slab caches - you can't have a slab cache for each used size. Also - you can't create a slab cache in interrupt context. > > This patch works for slab, slub and slob subsystems. The end of slab block > > can be found out with ksize (this patch renames it to __ksize). At the end > > of the block, we put a structure kmalloc_guard. This structure contains a > > magic number and a real size of the block - the exact size given to > > We already have a redzone structure to check for writes over the end of > the object. Lets use that. So, change all three slab subsystems to use that. Mikulas -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: implement kmalloc guard
On Fri, 5 Sep 2014, Mikulas Patocka wrote: > This patch adds a new option DEBUG_KMALLOC that makes it possible to > detect writes beyond the end of space allocated with kmalloc. Normally, > the kmalloc function rounds the size to the next power of two (there is > exception to this rule - sizes 96 and 192). Slab debugging detects only > writes beyond the end of the slab object, it is unable to detect writes > beyond the end of kmallocated size that fit into the slab object. > > The motivation for this patch was this: There was 6 year old bug in > dm-crypt (d49ec52ff6ddcda178fc2476a109cf1bd1fa19ed) where the driver would > write beyond the end of kmallocated space, but the bug went undetected > because the write would fit into the power-of-two-sized slab object. Only > recent changes to dm-crypt made the bug show up. There is similar problem > in the nx-crypto driver in the function nx_crypto_ctx_init - again, > because kmalloc rounds the size up to the next power of two, this bug went > undetected. The problem with the kmalloc array for debugging is inded that it is only for powers of two for efficiency purposes. In the debug situation we may not have the need for that efficiency. Maybe simply creating kmalloc arrays for each size will do the trick? > This patch works for slab, slub and slob subsystems. The end of slab block > can be found out with ksize (this patch renames it to __ksize). At the end > of the block, we put a structure kmalloc_guard. This structure contains a > magic number and a real size of the block - the exact size given to We already have a redzone structure to check for writes over the end of the object. Lets use that. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: implement kmalloc guard
On Fri, 5 Sep 2014, Mikulas Patocka wrote: This patch adds a new option DEBUG_KMALLOC that makes it possible to detect writes beyond the end of space allocated with kmalloc. Normally, the kmalloc function rounds the size to the next power of two (there is exception to this rule - sizes 96 and 192). Slab debugging detects only writes beyond the end of the slab object, it is unable to detect writes beyond the end of kmallocated size that fit into the slab object. The motivation for this patch was this: There was 6 year old bug in dm-crypt (d49ec52ff6ddcda178fc2476a109cf1bd1fa19ed) where the driver would write beyond the end of kmallocated space, but the bug went undetected because the write would fit into the power-of-two-sized slab object. Only recent changes to dm-crypt made the bug show up. There is similar problem in the nx-crypto driver in the function nx_crypto_ctx_init - again, because kmalloc rounds the size up to the next power of two, this bug went undetected. The problem with the kmalloc array for debugging is inded that it is only for powers of two for efficiency purposes. In the debug situation we may not have the need for that efficiency. Maybe simply creating kmalloc arrays for each size will do the trick? This patch works for slab, slub and slob subsystems. The end of slab block can be found out with ksize (this patch renames it to __ksize). At the end of the block, we put a structure kmalloc_guard. This structure contains a magic number and a real size of the block - the exact size given to We already have a redzone structure to check for writes over the end of the object. Lets use that. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: implement kmalloc guard
On Mon, 8 Sep 2014, Christoph Lameter wrote: On Fri, 5 Sep 2014, Mikulas Patocka wrote: This patch adds a new option DEBUG_KMALLOC that makes it possible to detect writes beyond the end of space allocated with kmalloc. Normally, the kmalloc function rounds the size to the next power of two (there is exception to this rule - sizes 96 and 192). Slab debugging detects only writes beyond the end of the slab object, it is unable to detect writes beyond the end of kmallocated size that fit into the slab object. The motivation for this patch was this: There was 6 year old bug in dm-crypt (d49ec52ff6ddcda178fc2476a109cf1bd1fa19ed) where the driver would write beyond the end of kmallocated space, but the bug went undetected because the write would fit into the power-of-two-sized slab object. Only recent changes to dm-crypt made the bug show up. There is similar problem in the nx-crypto driver in the function nx_crypto_ctx_init - again, because kmalloc rounds the size up to the next power of two, this bug went undetected. The problem with the kmalloc array for debugging is inded that it is only for powers of two for efficiency purposes. In the debug situation we may not have the need for that efficiency. Maybe simply creating kmalloc arrays for each size will do the trick? I don't know what you mean. If someone allocates 1 objects with sizes from 1 to 1, you can't have 1 slab caches - you can't have a slab cache for each used size. Also - you can't create a slab cache in interrupt context. This patch works for slab, slub and slob subsystems. The end of slab block can be found out with ksize (this patch renames it to __ksize). At the end of the block, we put a structure kmalloc_guard. This structure contains a magic number and a real size of the block - the exact size given to We already have a redzone structure to check for writes over the end of the object. Lets use that. So, change all three slab subsystems to use that. Mikulas -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slab: implement kmalloc guard
On Mon, 8 Sep 2014, Mikulas Patocka wrote: I don't know what you mean. If someone allocates 1 objects with sizes from 1 to 1, you can't have 1 slab caches - you can't have a slab cache for each used size. Also - you can't create a slab cache in interrupt context. Oh you can create them up front on bootup. And I think only the small sizes matter. Allocations =8K are pushed to the page allocator anyways. We already have a redzone structure to check for writes over the end of the object. Lets use that. So, change all three slab subsystems to use that. SLOB has no debugging features and I think that was intentional. We are trying to unify the debug checks etc. Some work on that would be appreciated. I think the kmalloc creation is already in slab_common.c -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
