lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 18 Jul 2013 14:37:15 -0700 From: Andrew Morton <akpm@...ux-foundation.org> To: Jan Kara <jack@...e.cz> Cc: LKML <linux-kernel@...r.kernel.org>, linux-mm@...ck.org, Jens Axboe <jaxboe@...ionio.com> Subject: Re: [PATCH RFC] lib: Make radix_tree_node_alloc() irq safe On Thu, 18 Jul 2013 15:09:32 +0200 Jan Kara <jack@...e.cz> wrote: > On Wed 17-07-13 16:12:00, Andrew Morton wrote: > > On Tue, 16 Jul 2013 19:06:30 +0200 Jan Kara <jack@...e.cz> wrote: > > > > BUG_ON(in_interrupt()) :) > Or maybe WARN_ON()... But it's not so easy :) Currently radix tree code > assumes that if gfp_mask doesn't have __GFP_WAIT set caller has performed > radix_tree_preload(). Clearly this will stop working for in-interrupt users > of radix tree. So how do we propagate the information from the caller of > radix_tree_insert() down to radix_tree_node_alloc() whether the preload has > been performed or not? Will we rely on in_interrupt() or use some special > gfp_mask bit? Well, it won't stop working. The interrupt-time radix_tree_node_alloc() call will try to grab a node from the cpu-local magazine and if that failed, will call kmem_cache_alloc(). Presumably the caller has passed in GFP_ATOMIC, so the kmem_cache_alloc() will use page reserves, which seems appropriate. This will mean that the interrupt-time node allocation will sometimes steal a preloaded node from process-context code. In the absolutely worst case, the process-context code will then need to try kmem_cache_alloc(), which will probably succeed anyway. It's not perfect - we'd prefer that process-context node allocations not get stolen in this fashion. That's easily fixed with --- a/lib/radix-tree.c~a +++ a/lib/radix-tree.c @@ -207,7 +207,10 @@ radix_tree_node_alloc(struct radix_tree_ struct radix_tree_node *ret = NULL; gfp_t gfp_mask = root_gfp_mask(root); - if (!(gfp_mask & __GFP_WAIT)) { + /* + * Lengthy comment goes here + */ + if (!(gfp_mask & __GFP_WAIT) && !in_interrupt()) { struct radix_tree_preload *rtp; /* But I don't know if it's worth it. > Secondly, CFQ has this unpleasant property that some functions are > sometimes called from interrupt context and sometimes not. So these > functions would have to check in what context they are called and either > perform preload or not. That's doable but it's going to be a bit ugly and > has to match the check in radix_tree_node_alloc() whether preload should be > used or not. So leaving the checking to the users of radix tree looks > fragile. mm... The CFQ code should be passing around a gfp_t anyway - GFP_NOIO or GFP_ATOMIC, depending on the calling context. So don't call radix_tree_preload() if it's GFP_ATOMIC. > So maybe we could just silently exit from radix_tree_preload() > when we are in_interrupt()? Or that. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists