Hi,
On Fri, 12 Apr 2013 14:39:23 +0800
Sha Zhengju <handai.szj@xxxxxxxxx> wrote:
> From: Sha Zhengju <handai.szj@xxxxxxxxxx>
>
> While writing memory.limit_in_bytes, a confusing result may happen:
>
> $ mkdir /memcg/test
> $ cat /memcg/test/memory.limit_in_bytes
> 9223372036854775807
> $ cat /memcg/test/memory.memsw.limit_in_bytes
> 9223372036854775807
> $ echo 18446744073709551614 > /memcg/test/memory.limit_in_bytes
> $ cat /memcg/test/memory.limit_in_bytes
> 0
>
> Strangely, the write successed and reset the limit to 0.
> The patch corrects RESOURCE_MAX and fixes this kind of overflow.
>
>
> Signed-off-by: Sha Zhengju <handai.szj@xxxxxxxxxx>
> Reported-by: Li Wenpeng < xingke.lwp@xxxxxxxxxx>
> Cc: Jie Liu <jeff.liu@xxxxxxxxxx>
> ---
> include/linux/res_counter.h | 2 +-
> kernel/res_counter.c | 8 +++++++-
> 2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/res_counter.h b/include/linux/res_counter.h
> index c230994..c2f01fc 100644
> --- a/include/linux/res_counter.h
> +++ b/include/linux/res_counter.h
> @@ -54,7 +54,7 @@ struct res_counter {
> struct res_counter *parent;
> };
>
> -#define RESOURCE_MAX (unsigned long long)LLONG_MAX
> +#define RESOURCE_MAX (unsigned long long)ULLONG_MAX
>
I don't think it's a good idea to change a user-visible value.
> /**
> * Helpers to interact with userspace
> diff --git a/kernel/res_counter.c b/kernel/res_counter.c
> index ff55247..6c35310 100644
> --- a/kernel/res_counter.c
> +++ b/kernel/res_counter.c
> @@ -195,6 +195,12 @@ int res_counter_memparse_write_strategy(const char *buf,
> if (*end != '\0')
> return -EINVAL;
>
> - *res = PAGE_ALIGN(*res);
> + /* Since PAGE_ALIGN is aligning up(the next page boundary),
> + * check the left space to avoid overflow to 0. */
> + if (RESOURCE_MAX - *res < PAGE_SIZE - 1)
> + *res = RESOURCE_MAX;
> + else
> + *res = PAGE_ALIGN(*res);
> +
Current interface seems strange because we can set a bigger value than
the value which means "unlimited".
So, how about some thing like:
if (*res > RESOURCE_MAX)
return -EINVAL;
if (*res > PAGE_ALIGN(RESOURCE_MAX) - PAGE_SIZE)
*res = RESOURCE_MAX;
else
*res = PAGE_ALIGN(*res);
?
> return 0;
> }
> --
> 1.7.9.5
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@xxxxxxxxx. For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>