linux-kernel - Re: [PATCH] cgroup/misc: Fix an overflow

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]

Date:   Mon, 17 Jul 2023 08:57:59 -1000
From:   Tejun Heo <tj@...nel.org>
To:     Jarkko Sakkinen <jarkko@...nel.org>
Cc:     Haitao Huang <haitao.huang@...ux.intel.com>,
        dave.hansen@...ux.intel.com, linux-kernel@...r.kernel.org,
        linux-sgx@...r.kernel.org, cgroups@...r.kernel.org,
        Zefan Li <lizefan.x@...edance.com>,
        Johannes Weiner <hannes@...xchg.org>, vipinsh@...gle.com,
        kai.huang@...el.com, reinette.chatre@...el.com,
        zhiquan1.li@...el.com, kristen@...ux.intel.com
Subject: Re: [PATCH] cgroup/misc: Fix an overflow

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@...ux.intel.com>
> >
> > [1] https://lore.kernel.org/linux-sgx/20230712230202.47929-1-haitao.huang@linux.intel.com/
> > ---
> >  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?

Thanks.

-- 
tejun