Re: [PATCH] cgroup/misc: Fix an overflow

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 17 Jul 2023 14:01:03 -0500, Haitao Huang <haitao.huang@xxxxxxxxxxxxxxx> wrote:

On Mon, 17 Jul 2023 13:57:59 -0500, Tejun Heo <tj@xxxxxxxxxx> wrote:

On Mon, Jul 17, 2023 at 06:55:32PM +0000, Jarkko Sakkinen wrote:
On Mon Jul 17, 2023 at 6:47 PM UTC, Haitao Huang wrote:
> The variable 'new_usage' in misc_cg_try_charge() may overflow if it
> becomes above INT_MAX. This was observed when I implement the new SGX
> EPC cgroup[1] as a misc cgroup and test on a platform with large SGX EPC
> sizes.
>
> Change type of new_usage to long from int and check overflow.
>
> Fixes: a72232eabdfcf ("cgroup: Add misc cgroup controller")
> Signed-off-by: Haitao Huang <haitao.huang@xxxxxxxxxxxxxxx>
>
> [1] https://lore.kernel.org/linux-sgx/20230712230202.47929-1-haitao.huang@xxxxxxxxxxxxxxx/
> ---
>  kernel/cgroup/misc.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/cgroup/misc.c b/kernel/cgroup/misc.c
> index fe3e8a0eb7ed..ff9f900981a3 100644
> --- a/kernel/cgroup/misc.c
> +++ b/kernel/cgroup/misc.c
> @@ -143,7 +143,7 @@ int misc_cg_try_charge(enum misc_res_type type, struct misc_cg *cg,
>  	struct misc_cg *i, *j;
>  	int ret;
>  	struct misc_res *res;
> -	int new_usage;
> +	long new_usage;
>
> if (!(valid_type(type) && cg && READ_ONCE(misc_res_capacity[type])))
>  		return -EINVAL;
> @@ -153,10 +153,10 @@ int misc_cg_try_charge(enum misc_res_type type, struct misc_cg *cg,
>
>  	for (i = cg; i; i = parent_misc(i)) {
>  		res = &i->res[type];
> -

This is extra noise in the patch, please remove the change.

Lemme just revert it. Haitao, can you instead make the resource counters and
all related variables explicit 64bit instead?


Will do.

Actually, we are using atomic_long_t for 'current' which is the same width as long defined by arch/compiler. So new_usage should be long to be consistent?

ditto for event counter. Only max is plain unsigned long but I think it is also OK as it only compared with 'current' without any arithmetic ops involved.
Did I miss something here?
Thanks
Haitao



[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux