Re: [RESEND PATCH 1/1] mm/percpu.c: correct max_distance calculation for pcpu_embed_first_chunk()

[Tejun Heo]

Hello,

On Sat, Sep 24, 2016 at 07:20:49AM +0800, zijun_hu wrote:
> it is error to represent the max range max_distance spanned by all the
> group areas as the offset of the highest group area plus unit size in
> pcpu_embed_first_chunk(), it should equal to the offset plus the size
> of the highest group area
> 
> in order to fix this issue,let us find the highest group area who has the
> biggest base address among all the ones, then max_distance is formed by
> add it's offset and size value

 [PATCH] percpu: fix max_distance calculation in pcpu_embed_first_chunk()

 pcpu_embed_first_chunk() calculates the range a percpu chunk spans
 into max_distance and uses it to ensure that a chunk is not too big
 compared to the total vmalloc area.  However, during calculation, it
 used incorrect top address by adding a unit size to the higest
 group's base address.

 This can make the calculated max_distance slightly smaller than the
 actual distance although given the scale of values involved the error
 is very unlikely to have an actual impact.

 Fix this issue by adding the group's size instead of a unit size.

> the type of variant max_distance is changed from size_t to unsigned long
> to prevent potential overflow

This doesn't make any sense.  All the values involved are valid
addresses (or +1 of it), they can't overflow and size_t is the same
size as ulong.

> @@ -2025,17 +2026,18 @@ int __init pcpu_embed_first_chunk(size_t reserved_size, size_t dyn_size,
>  	}
>  
>  	/* base address is now known, determine group base offsets */
> -	max_distance = 0;
> +	i = 0;
>  	for (group = 0; group < ai->nr_groups; group++) {
>  		ai->groups[group].base_offset = areas[group] - base;
> -		max_distance = max_t(size_t, max_distance,
> -				     ai->groups[group].base_offset);
> +		if (areas[group] > areas[i])
> +			i = group;
>  	}
> -	max_distance += ai->unit_size;
> +	max_distance = ai->groups[i].base_offset +
> +		(unsigned long)ai->unit_size * ai->groups[i].nr_units;

I don't think you need ulong cast here.

Thanks.

-- 
tejun