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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <alpine.DEB.2.00.1012272241200.23315@chino.kir.corp.google.com>
Date:	Mon, 27 Dec 2010 23:34:45 -0800 (PST)
From:	David Rientjes <rientjes@...gle.com>
To:	Shaohui Zheng <shaohui.zheng@...el.com>,
	Andrew Morton <akpm@...ux-foundation.org>
cc:	linux-mm@...ck.org, linux-kernel@...r.kernel.org,
	haicheng.li@...ux.intel.com, lethal@...ux-sh.org,
	Andi Kleen <ak@...ux.intel.com>, dave@...ux.vnet.ibm.com,
	Greg Kroah-Hartman <gregkh@...e.de>,
	Haicheng Li <haicheng.li@...el.com>
Subject: Re: [3/7, v9] NUMA Hotplug Emulator: Add node hotplug emulation

On Wed, 22 Dec 2010, Andrew Morton wrote:

> > Index: linux-hpe4/mm/memory_hotplug.c
> > ===================================================================
> > --- linux-hpe4.orig/mm/memory_hotplug.c	2010-11-30 12:40:43.757622001 +0800
> > +++ linux-hpe4/mm/memory_hotplug.c	2010-11-30 14:02:33.877622002 +0800
> > @@ -924,3 +924,63 @@
> >  }
> >  #endif /* CONFIG_MEMORY_HOTREMOVE */
> >  EXPORT_SYMBOL_GPL(remove_memory);
> > +
> > +#ifdef CONFIG_DEBUG_FS
> > +#include <linux/debugfs.h>
> > +
> > +static struct dentry *memhp_debug_root;
> > +
> > +static ssize_t add_node_store(struct file *file, const char __user *buf,
> > +				size_t count, loff_t *ppos)
> > +{
> > +	nodemask_t mask;
> 
> NODEMASK_ALLOC()?
> 
> > +	u64 start, size;
> > +	char buffer[64];
> > +	char *p;
> > +	int nid;
> > +	int ret;
> > +
> > +	memset(buffer, 0, sizeof(buffer));
> > +	if (count > sizeof(buffer) - 1)
> > +		count = sizeof(buffer) - 1;
> 
> This will cause the write to return a smaller number than `count': a
> short write.  Some userspace code may then decide to write the
> remainder of the data (whcih is the correct way to use the write()
> syscall).
> 
> Could be a bit dangerous, and perhaps simply declaring an error if too
> much data was written would be a better approach.
> 
> > +	if (copy_from_user(buffer, buf, count))
> > +		return -EFAULT;
> > +
> > +	size = memparse(buffer, &p);
> > +	if (size < (PAGES_PER_SECTION << PAGE_SHIFT))
> 
> PAGES_PER_SECTION has type unsigned long, so the rhs of this comparison
> might overflow on 32-bit, should anyone ever try to use this code on
> 32-bit.
> 
> otoh the compiler might do it as 64-bit because the lhs is 64-bit.  Not
> sure.
> 
> > +		return -EINVAL;
> > +	if (*p != '@')
> > +		return -EINVAL;
> > +
> > +	start = simple_strtoull(p + 1, NULL, 0);
> 
> You disagreed with checkpatch?
> 
> > +	nodes_andnot(mask, node_possible_map, node_online_map);
> > +	nid = first_node(mask);
> > +	if (nid == MAX_NUMNODES)
> > +		return -ENOMEM;
> > +
> > +	ret = add_memory(nid, start, size);
> > +	return ret ? ret : count;
> > +}
> > +
> > +static const struct file_operations add_node_file_ops = {
> > +	.write		= add_node_store,
> > +	.llseek		= generic_file_llseek,
> > +};
> > +
> > +static int __init node_debug_init(void)
> > +{
> > +	if (!memhp_debug_root)
> > +		memhp_debug_root = debugfs_create_dir("mem_hotplug", NULL);
> > +	if (!memhp_debug_root)
> > +		return -ENOMEM;
> > +
> > +	if (!debugfs_create_file("add_node", S_IWUSR, memhp_debug_root,
> > +			NULL, &add_node_file_ops))
> > +		return -ENOMEM;
> > +
> > +	return 0;
> > +}
> > +
> > +module_init(node_debug_init);
> > +#endif /* CONFIG_DEBUG_FS */

Shaohui, I'll reply to this message with an updated version of this patch 
to address Andrew's comments.  You can merge it into your series or Andrew 
can take it seperately (although it doesn't do much good without "x86: add 
numa=possible command line option" unless you have hotpluggable SRAT 
entries and CONFIG_ACPI_NUMA).
--
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

Powered by Openwall GNU/*/Linux Powered by OpenVZ