lkml.org 
[lkml]   [2014]   [May]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Subject[PATCH 1/1] mem-hotplug: Avoid illegal state prefixed with legal state when changing state of memory_block.
Date
We use the following command to online a memory_block:

echo online|online_kernel|online_movable > /sys/devices/system/memory/memoryXXX/state

But, if we typed "online_movbale" by mistake (typo, not "online_movable"), it will be
recognized as "online", and it will online the memory block successfully. "online" command
will put the memory block into the same zone as it was in before last offlined, which may
be ZONE_NORMAL, not ZONE_MOVABLE. Since it succeeds without any warning, it may confuse
users.

Furthermore, if we do the following:

echo online_fhsjkghfkd > /sys/devices/system/memory/memoryXXX/state
the block will also be onlined.

echo offline_ohjkjewrj > /sys/devices/system/memory/memoryXXX/state
the block will also be offlined.

This is because the following code in store_mem_state() does not compare the whole string,
but only the prefix of the string.

store_mem_state()
{
......
328 if (!strncmp(buf, "online_kernel", min_t(int, count, 13)))

Here, only compare the first 13 letters of the string. If we give "online_kernelXXXXXX",
it will be recognized as online_kernel, which is incorrect.

329 online_type = ONLINE_KERNEL;
330 else if (!strncmp(buf, "online_movable", min_t(int, count, 14)))

We have the same problem here,

331 online_type = ONLINE_MOVABLE;
332 else if (!strncmp(buf, "online", min_t(int, count, 6)))

here,

(Here is more problematic. If we give online_movalbe, which is a typo of online_movable,
it will be recognized as online without noticing the author.)

333 online_type = ONLINE_KEEP;
334 else if (!strncmp(buf, "offline", min_t(int, count, 7)))

and here.

335 online_type = -1;
336 else {
337 ret = -EINVAL;
338 goto err;
339 }
......
}

This patch fix this problem by checking if the length of the string is 6,7,13,14, and then
compare the whole string.

And in store_mem_state(), the parameter count passed from user space includes '\0' in the
end of the string, so the real length of the string is count-1.

Reported-by: Hu Tao <hutao@cn.fujitsu.com>
Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com>
---
drivers/base/memory.c | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/base/memory.c b/drivers/base/memory.c
index bece691..3ff2adb 100644
--- a/drivers/base/memory.c
+++ b/drivers/base/memory.c
@@ -319,19 +319,27 @@ store_mem_state(struct device *dev,
struct device_attribute *attr, const char *buf, size_t count)
{
struct memory_block *mem = to_memory_block(dev);
- int ret, online_type;
+ int ret, online_type, len;

ret = lock_device_hotplug_sysfs();
if (ret)
return ret;

- if (!strncmp(buf, "online_kernel", min_t(int, count, 13)))
+ /*
+ * count passed from user space includes \0, so the real length
+ * is count-1.
+ */
+ len = count - 1;
+
+ if (len == strlen("online_kernel") &&
+ !strncmp(buf, "online_kernel", len))
online_type = ONLINE_KERNEL;
- else if (!strncmp(buf, "online_movable", min_t(int, count, 14)))
+ else if (len == strlen("online_movable") &&
+ !strncmp(buf, "online_movable", len))
online_type = ONLINE_MOVABLE;
- else if (!strncmp(buf, "online", min_t(int, count, 6)))
+ else if (len == strlen("online") && !strncmp(buf, "online", len))
online_type = ONLINE_KEEP;
- else if (!strncmp(buf, "offline", min_t(int, count, 7)))
+ else if (len == strlen("offline") && !strncmp(buf, "offline", len))
online_type = -1;
else {
ret = -EINVAL;
--
1.8.3.1


\
 
 \ /
  Last update: 2014-05-16 05:01    [W:0.050 / U:0.184 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site