linux-kernel - Re: [PATCH v5 4/5] drm/i915/gvt: Dmabuf support for GVT-g

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]

Message-ID: <1495548049.30747.6.camel@redhat.com>
Date:   Tue, 23 May 2017 16:00:49 +0200
From:   Gerd Hoffmann <kraxel@...hat.com>
To:     Xiaoguang Chen <xiaoguang.chen@...el.com>,
        alex.williamson@...hat.com, intel-gfx@...ts.freedesktop.org,
        linux-kernel@...r.kernel.org, zhenyuw@...ux.intel.com,
        zhiyuan.lv@...el.com, intel-gvt-dev@...ts.freedesktop.org,
        zhi.a.wang@...el.com, kevin.tian@...el.com
Subject: Re: [PATCH v5 4/5] drm/i915/gvt: Dmabuf support for GVT-g

  Hi,

> +	} else if (plane_id == INTEL_GVT_PLANE_CURSOR) {
> +		c = &pipe->cursor;
> +		if (c != NULL) {
> +			info->start = c->base;
> +			info->width = c->width;
> +			info->height = c->height;
> +			info->stride = c->width * (c->bpp / 8);
> +			info->drm_format_mod = 0;
> +			info->x_pos = c->x_pos;
> +			info->y_pos = c->y_pos;
> +			info->size = (((info->stride * c->height *
> c->bpp) / 8)
> +					+ (PAGE_SIZE - 1)) >>
> PAGE_SHIFT;

Leaves info->drm_format unset.

> +struct intel_vgpu_plane_info {
> +	uint32_t plane_id;
> +	uint32_t drm_format;
> +	uint32_t width;
> +	uint32_t height;
> +	uint32_t stride;
> +	uint32_t start;
> +	uint32_t x_pos;
> +	uint32_t y_pos;
> +	uint32_t size;
> +	uint64_t drm_format_mod;
> +};

drm_format_mod is unaligned.  Should be avoided, otherwise the struct
layout is different on i386 and x86_64.

Patch 5/5 moves around this struct.  Better have a separate patch
adding the structs and ioctls, order it before this one, so you don't
have to rename the stuff just added ...

> +struct intel_vgpu_dmabuf {
> +	uint32_t fd;
> +	struct intel_vgpu_plane_info plane_info;
> +};

Has alignment problems too.

cheers,
  Gerd