linux-kernel - Re: [PATCH] drm/i915/gvt: Use ARRAY

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Mon, 18 Jun 2018 09:20:32 -0700
From:   Joe Perches <joe@...ches.com>
To:     rajan.vaja@...il.com, zhenyuw@...ux.intel.com,
        zhi.a.wang@...el.com, jani.nikula@...ux.intel.com,
        joonas.lahtinen@...ux.intel.com, rodrigo.vivi@...el.com,
        airlied@...ux.ie
Cc:     intel-gvt-dev@...ts.freedesktop.org,
        dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] drm/i915/gvt: Use ARRAY_SIZE macro

On Mon, 2018-06-18 at 13:18 +0530, rajan.vaja@...il.com wrote:
> Use ARRAY_SIZE instead of dividing sizeof array with sizeof
> an element. This fixes below warning reported by Coccinelle:
> drivers/gpu/drm//i915/gvt/vgpu.c:122:30-31: WARNING: Use ARRAY_SIZE
[]
> diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c b/drivers/gpu/drm/i915/gvt/vgpu.c
[]
> @@ -119,7 +119,7 @@ int intel_gvt_init_vgpu_types(struct intel_gvt *gvt)
>  	 */
>  	low_avail = gvt_aperture_sz(gvt) - HOST_LOW_GM_SIZE;
>  	high_avail = gvt_hidden_sz(gvt) - HOST_HIGH_GM_SIZE;
> -	num_types = sizeof(vgpu_types) / sizeof(vgpu_types[0]);
> +	num_types = ARRAY_SIZE(vgpu_types);
>  
>  	gvt->types = kzalloc(num_types * sizeof(struct intel_vgpu_type),
>  			     GFP_KERNEL);

It seems you are not using -next as this alloc has
already been changed to kcalloc.

It'd be better to get rid of num_types altogether and
use ARRAY_SIZE everywhere instead.

There are also string overflow possibilities in the
function where sprintf should probably use snprintf.

Perhaps:
'
int intel_gvt_init_vgpu_types(struct intel_gvt *gvt)
{
	unsigned int i, low_avail, high_avail;
	unsigned int min_low;

	/* vGPU type name is defined as GVTg_Vx_y which contains
	 * physical GPU generation type (e.g V4 as BDW server, V5 as
	 * SKL server).
	 *
	 * Depend on physical SKU resource, might see vGPU types like
	 * GVTg_V4_8, GVTg_V4_4, GVTg_V4_2, etc. We can create
	 * different types of vGPU on same physical GPU depending on
	 * available resource. Each vGPU type will have "avail_instance"
	 * to indicate how many vGPU instance can be created for this
	 * type.
	 *
	 */
	low_avail = gvt_aperture_sz(gvt) - HOST_LOW_GM_SIZE;
	high_avail = gvt_hidden_sz(gvt) - HOST_HIGH_GM_SIZE;

	gvt->types = kcalloc(ARRAY_SIZE(vgpu_types),
			     sizeof(struct intel_vgpu_type), GFP_KERNEL);
	if (!gvt->types)
		return -ENOMEM;

	min_low = MB_TO_BYTES(32);
	for (i = 0; i < ARRAY_SIZE(vgpu_types); i++) {
		if (low_avail / vgpu_types[i].low_mm == 0)
			break;

		gvt->types[i].low_gm_size = vgpu_types[i].low_mm;
		gvt->types[i].high_gm_size = vgpu_types[i].high_mm;
		gvt->types[i].fence = vgpu_types[i].fence;

		if (vgpu_types[i].weight < 1 ||
		    vgpu_types[i].weight > VGPU_MAX_WEIGHT)
			return -EINVAL;

		gvt->types[i].weight = vgpu_types[i].weight;
		gvt->types[i].resolution = vgpu_types[i].edid;
		gvt->types[i].avail_instance =
			min(low_avail / vgpu_types[i].low_mm,
			    high_avail / vgpu_types[i].high_mm);

		if (IS_GEN8(gvt->dev_priv))
			snprintf(gvt->types[i].name, sizeof(gvt->types[i].name),
				 "GVTg_V4_%s", vgpu_types[i].name);
		else if (IS_GEN9(gvt->dev_priv))
			snprintf(gvt->types[i].name, sizeof(gvt->types[i].name),
				 "GVTg_V5_%s", vgpu_types[i].name);

		gvt_dbg_core("type[%d]: %s avail %u low %u high %u fence %u weight %u res %s\n",
			     i, gvt->types[i].name,
			     gvt->types[i].avail_instance,
			     gvt->types[i].low_gm_size,
			     gvt->types[i].high_gm_size, gvt->types[i].fence,
			     gvt->types[i].weight,
			     vgpu_edid_str(gvt->types[i].resolution));
	}

	gvt->num_types = i;

	return 0;
}