* [PATCH 2/3] memcg: check more strictly to avoid PAGE_ALIGN wrapped to 0
@ 2013-05-05 15:43 Sha Zhengju
2013-05-07 14:04 ` Michal Hocko
0 siblings, 1 reply; 3+ messages in thread
From: Sha Zhengju @ 2013-05-05 15:43 UTC (permalink / raw)
To: cgroups, linux-mm; +Cc: nishimura, akpm, mhocko, jeff.liu, Sha Zhengju
Since PAGE_ALIGN is aligning up(the next page boundary), this can
prevent input values wrapped to 0 and cause strange result to user.
This patch also rename the second arg of
res_counter_memparse_write_strategy() to 'resp' and add a local
variable 'res' to save the too often dereferences. Thanks Andrew
for pointing it out!
Signed-off-by: Sha Zhengju <handai.szj@taobao.com>
Reported-by: Li Wenpeng <xingke.lwp@taobao.com>
---
kernel/res_counter.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/kernel/res_counter.c b/kernel/res_counter.c
index 3f0417f..be8ddda 100644
--- a/kernel/res_counter.c
+++ b/kernel/res_counter.c
@@ -178,23 +178,29 @@ u64 res_counter_read_u64(struct res_counter *counter, int member)
#endif
int res_counter_memparse_write_strategy(const char *buf,
- unsigned long long *res)
+ unsigned long long *resp)
{
char *end;
+ unsigned long long res;
/* return RES_COUNTER_MAX(unlimited) if "-1" is specified */
if (*buf == '-') {
- *res = simple_strtoull(buf + 1, &end, 10);
- if (*res != 1 || *end != '\0')
+ res = simple_strtoull(buf + 1, &end, 10);
+ if (res != 1 || *end != '\0')
return -EINVAL;
- *res = RES_COUNTER_MAX;
+ *resp = RES_COUNTER_MAX;
return 0;
}
- *res = memparse(buf, &end);
+ res = memparse(buf, &end);
if (*end != '\0')
return -EINVAL;
- *res = PAGE_ALIGN(*res);
+ if (PAGE_ALIGN(res) >= res)
+ res = PAGE_ALIGN(res);
+ else
+ res = RES_COUNTER_MAX; /* avoid PAGE_ALIGN wrapping to zero */
+
+ *resp = res;
return 0;
}
--
1.7.9.5
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 2/3] memcg: check more strictly to avoid PAGE_ALIGN wrapped to 0
2013-05-05 15:43 [PATCH 2/3] memcg: check more strictly to avoid PAGE_ALIGN wrapped to 0 Sha Zhengju
@ 2013-05-07 14:04 ` Michal Hocko
2013-05-07 15:41 ` Sha Zhengju
0 siblings, 1 reply; 3+ messages in thread
From: Michal Hocko @ 2013-05-07 14:04 UTC (permalink / raw)
To: Sha Zhengju; +Cc: cgroups, linux-mm, nishimura, akpm, jeff.liu, Sha Zhengju
On Sun 05-05-13 23:43:10, Sha Zhengju wrote:
> Since PAGE_ALIGN is aligning up(the next page boundary), this can
> prevent input values wrapped to 0 and cause strange result to user.
I guess you wanted to say that it can cause an overflow, right?
"
Since PAGE_ALIGN is aligning up (to the next page boundary), this can
cause an overflow to 0 if >= ULLONG_MAX-4094 value is given in the
buffer.
"
>
> This patch also rename the second arg of
> res_counter_memparse_write_strategy() to 'resp' and add a local
> variable 'res' to save the too often dereferences. Thanks Andrew
> for pointing it out!
Again, it would be nicer to have this cleanup in a separate patch.
> Signed-off-by: Sha Zhengju <handai.szj@taobao.com>
> Reported-by: Li Wenpeng <xingke.lwp@taobao.com>
Acked-by: Michal Hocko <mhocko@suse.cz>
We have this bug since ever and nobody has noticed so nobody seems to
use
> ---
> kernel/res_counter.c | 18 ++++++++++++------
> 1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/kernel/res_counter.c b/kernel/res_counter.c
> index 3f0417f..be8ddda 100644
> --- a/kernel/res_counter.c
> +++ b/kernel/res_counter.c
> @@ -178,23 +178,29 @@ u64 res_counter_read_u64(struct res_counter *counter, int member)
> #endif
>
> int res_counter_memparse_write_strategy(const char *buf,
> - unsigned long long *res)
> + unsigned long long *resp)
> {
> char *end;
> + unsigned long long res;
>
> /* return RES_COUNTER_MAX(unlimited) if "-1" is specified */
> if (*buf == '-') {
> - *res = simple_strtoull(buf + 1, &end, 10);
> - if (*res != 1 || *end != '\0')
> + res = simple_strtoull(buf + 1, &end, 10);
> + if (res != 1 || *end != '\0')
> return -EINVAL;
> - *res = RES_COUNTER_MAX;
> + *resp = RES_COUNTER_MAX;
> return 0;
> }
>
> - *res = memparse(buf, &end);
> + res = memparse(buf, &end);
> if (*end != '\0')
> return -EINVAL;
>
> - *res = PAGE_ALIGN(*res);
> + if (PAGE_ALIGN(res) >= res)
> + res = PAGE_ALIGN(res);
> + else
> + res = RES_COUNTER_MAX; /* avoid PAGE_ALIGN wrapping to zero */
> +
> + *resp = res;
> return 0;
> }
> --
> 1.7.9.5
>
--
Michal Hocko
SUSE Labs
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 2/3] memcg: check more strictly to avoid PAGE_ALIGN wrapped to 0
2013-05-07 14:04 ` Michal Hocko
@ 2013-05-07 15:41 ` Sha Zhengju
0 siblings, 0 replies; 3+ messages in thread
From: Sha Zhengju @ 2013-05-07 15:41 UTC (permalink / raw)
To: Michal Hocko
Cc: Cgroups, linux-mm, Daisuke Nishimura, Andrew Morton, jeff.liu,
Sha Zhengju
On Tue, May 7, 2013 at 10:04 PM, Michal Hocko <mhocko@suse.cz> wrote:
> On Sun 05-05-13 23:43:10, Sha Zhengju wrote:
>> Since PAGE_ALIGN is aligning up(the next page boundary), this can
>> prevent input values wrapped to 0 and cause strange result to user.
>
> I guess you wanted to say that it can cause an overflow, right?
> "
> Since PAGE_ALIGN is aligning up (to the next page boundary), this can
> cause an overflow to 0 if >= ULLONG_MAX-4094 value is given in the
> buffer.
> "
Yes!
>>
>> This patch also rename the second arg of
>> res_counter_memparse_write_strategy() to 'resp' and add a local
>> variable 'res' to save the too often dereferences. Thanks Andrew
>> for pointing it out!
>
> Again, it would be nicer to have this cleanup in a separate patch.
Okay.
>
>> Signed-off-by: Sha Zhengju <handai.szj@taobao.com>
>> Reported-by: Li Wenpeng <xingke.lwp@taobao.com>
>
> Acked-by: Michal Hocko <mhocko@suse.cz>
>
> We have this bug since ever and nobody has noticed so nobody seems to
> use
Yeah, that's rarely occur, but we happened to run into it.
Thank you for the comments!
>
>> ---
>> kernel/res_counter.c | 18 ++++++++++++------
>> 1 file changed, 12 insertions(+), 6 deletions(-)
>>
>> diff --git a/kernel/res_counter.c b/kernel/res_counter.c
>> index 3f0417f..be8ddda 100644
>> --- a/kernel/res_counter.c
>> +++ b/kernel/res_counter.c
>> @@ -178,23 +178,29 @@ u64 res_counter_read_u64(struct res_counter *counter, int member)
>> #endif
>>
>> int res_counter_memparse_write_strategy(const char *buf,
>> - unsigned long long *res)
>> + unsigned long long *resp)
>> {
>> char *end;
>> + unsigned long long res;
>>
>> /* return RES_COUNTER_MAX(unlimited) if "-1" is specified */
>> if (*buf == '-') {
>> - *res = simple_strtoull(buf + 1, &end, 10);
>> - if (*res != 1 || *end != '\0')
>> + res = simple_strtoull(buf + 1, &end, 10);
>> + if (res != 1 || *end != '\0')
>> return -EINVAL;
>> - *res = RES_COUNTER_MAX;
>> + *resp = RES_COUNTER_MAX;
>> return 0;
>> }
>>
>> - *res = memparse(buf, &end);
>> + res = memparse(buf, &end);
>> if (*end != '\0')
>> return -EINVAL;
>>
>> - *res = PAGE_ALIGN(*res);
>> + if (PAGE_ALIGN(res) >= res)
>> + res = PAGE_ALIGN(res);
>> + else
>> + res = RES_COUNTER_MAX; /* avoid PAGE_ALIGN wrapping to zero */
>> +
>> + *resp = res;
>> return 0;
>> }
>> --
>> 1.7.9.5
>>
>
> --
> Michal Hocko
> SUSE Labs
--
Thanks,
Sha
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-05-07 15:41 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-05-05 15:43 [PATCH 2/3] memcg: check more strictly to avoid PAGE_ALIGN wrapped to 0 Sha Zhengju
2013-05-07 14:04 ` Michal Hocko
2013-05-07 15:41 ` Sha Zhengju
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox