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: Tue, 28 Aug 2018 14:04:18 +0000 From: Pasha Tatashin <Pavel.Tatashin@...rosoft.com> To: Oscar Salvador <osalvador@...hadventures.net>, "akpm@...ux-foundation.org" <akpm@...ux-foundation.org> CC: "mhocko@...e.com" <mhocko@...e.com>, "vbabka@...e.cz" <vbabka@...e.cz>, "dan.j.williams@...el.com" <dan.j.williams@...el.com>, "yasu.isimatu@...il.com" <yasu.isimatu@...il.com>, "jonathan.cameron@...wei.com" <jonathan.cameron@...wei.com>, "david@...hat.com" <david@...hat.com>, "linux-mm@...ck.org" <linux-mm@...ck.org>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, Oscar Salvador <osalvador@...e.de> Subject: Re: [PATCH v4 3/4] mm/memory_hotplug: Define nodemask_t as a stack variable On 8/17/18 5:00 AM, Oscar Salvador wrote: > From: Oscar Salvador <osalvador@...e.de> > > Currently, unregister_mem_sect_under_nodes() tries to allocate a nodemask_t > in order to check whithin the loop which nodes have already been unlinked, > so we do not repeat the operation on them. > > NODEMASK_ALLOC calls kmalloc() if NODES_SHIFT > 8, otherwise > it just declares a nodemask_t variable whithin the stack. > > Since kmalloc() can fail, we actually check whether NODEMASK_ALLOC failed > or not, and we return -ENOMEM accordingly. > remove_memory_section() does not check for the return value though. > It is pretty rare that such a tiny allocation can fail, but if it does, > we will be left with dangled symlinks under /sys/devices/system/node/, > since the mem_blk's directories will be removed no matter what > unregister_mem_sect_under_nodes() returns. > > One way to solve this is to check whether unlinked_nodes is NULL or not. > In the case it is not, we can just use it as before, but if it is NULL, > we can just skip the node_test_and_set check, and call sysfs_remove_link() > unconditionally. > This is harmless as sysfs_remove_link() backs off somewhere down the chain > in case the link has already been removed. > This method was presented in v3 of the path [1]. > > But since the maximum number of nodes we can have is 1024, > when NODES_SHIFT = 10, that gives us a nodemask_t of 128 bytes. > Although everything depends on how deep the stack is, I think we can afford > to define the nodemask_t variable whithin the stack. > > That simplifies the code, and we do not need to worry about untested error > code paths. > > If we see that this causes troubles with the stack, we can always return to [1]. > > Signed-off-by: Oscar Salvador <osalvador@...e.de> LGTM: Reviewed-by: Pavel Tatashin <pavel.tatashin@...rosoft.com> Thank you, Pavel
Powered by blists - more mailing lists