| 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
| ||
|
Message-ID: <050b7fac-1ebd-46a0-840e-2df8c82136ae@amd.com>
Date: Mon, 16 Dec 2024 14:38:36 -0600
From: Mario Limonciello <mario.limonciello@....com>
To: Lizhi Hou <lizhi.hou@....com>, ogabbay@...nel.org,
quic_jhugo@...cinc.com, dri-devel@...ts.freedesktop.org
Cc: linux-kernel@...r.kernel.org, min.ma@....com, max.zhen@....com,
sonal.santan@....com, king.tam@....com
Subject: Re: [PATCH V3 5/5] accel/amdxdna: Add zero check for pad in ioctl
input structures
On 12/13/2024 17:29, Lizhi Hou wrote:
> For input structures, it is better to check if the pad is zero.
> Thus, the pad bytes might be usable in the future.
>
IIRC you should pick up:
Suggested-by: Jeffrey Hugo <quic_jhugo@...cinc.com>
> Signed-off-by: Lizhi Hou <lizhi.hou@....com>
> ---
> drivers/accel/amdxdna/aie2_ctx.c | 3 +++
> drivers/accel/amdxdna/aie2_message.c | 3 +++
> drivers/accel/amdxdna/amdxdna_ctx.c | 6 ++++++
> drivers/accel/amdxdna/amdxdna_gem.c | 2 +-
> include/uapi/drm/amdxdna_accel.h | 11 +++++------
> 5 files changed, 18 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/accel/amdxdna/aie2_ctx.c b/drivers/accel/amdxdna/aie2_ctx.c
> index cdeead75c6f5..9facf45818f9 100644
> --- a/drivers/accel/amdxdna/aie2_ctx.c
> +++ b/drivers/accel/amdxdna/aie2_ctx.c
> @@ -690,6 +690,9 @@ static int aie2_hwctx_cu_config(struct amdxdna_hwctx *hwctx, void *buf, u32 size
> int ret;
>
> XDNA_DBG(xdna, "Config %d CU to %s", config->num_cus, hwctx->name);
> + if (XDNA_MBZ_DBG(xdna, config->pad, sizeof(config->pad)))
> + return -EINVAL;
> +
> if (hwctx->status != HWCTX_STAT_INIT) {
> XDNA_ERR(xdna, "Not support re-config CU");
> return -EINVAL;
> diff --git a/drivers/accel/amdxdna/aie2_message.c b/drivers/accel/amdxdna/aie2_message.c
> index b2ca78cfd0a7..9e2c9a44f76a 100644
> --- a/drivers/accel/amdxdna/aie2_message.c
> +++ b/drivers/accel/amdxdna/aie2_message.c
> @@ -395,6 +395,9 @@ int aie2_config_cu(struct amdxdna_hwctx *hwctx)
> for (i = 0; i < hwctx->cus->num_cus; i++) {
> struct amdxdna_cu_config *cu = &hwctx->cus->cu_configs[i];
>
> + if (XDNA_MBZ_DBG(xdna, cu->pad, sizeof(cu->pad)))
> + return -EINVAL;
> +
> gobj = drm_gem_object_lookup(hwctx->client->filp, cu->cu_bo);
> if (!gobj) {
> XDNA_ERR(xdna, "Lookup GEM object failed");
> diff --git a/drivers/accel/amdxdna/amdxdna_ctx.c b/drivers/accel/amdxdna/amdxdna_ctx.c
> index 324f35c43f6c..d11b1c83d9c3 100644
> --- a/drivers/accel/amdxdna/amdxdna_ctx.c
> +++ b/drivers/accel/amdxdna/amdxdna_ctx.c
> @@ -243,6 +243,9 @@ int amdxdna_drm_destroy_hwctx_ioctl(struct drm_device *dev, void *data, struct d
> struct amdxdna_hwctx *hwctx;
> int ret = 0, idx;
>
> + if (XDNA_MBZ_DBG(xdna, &args->pad, sizeof(args->pad)))
> + return -EINVAL;
> +
> if (!drm_dev_enter(dev, &idx))
> return -ENODEV;
>
> @@ -277,6 +280,9 @@ int amdxdna_drm_config_hwctx_ioctl(struct drm_device *dev, void *data, struct dr
> void *buf;
> u64 val;
>
> + if (XDNA_MBZ_DBG(xdna, &args->pad, sizeof(args->pad)))
> + return -EINVAL;
> +
> if (!xdna->dev_info->ops->hwctx_config)
> return -EOPNOTSUPP;
>
> diff --git a/drivers/accel/amdxdna/amdxdna_gem.c b/drivers/accel/amdxdna/amdxdna_gem.c
> index 4dfeca306d98..606433d73236 100644
> --- a/drivers/accel/amdxdna/amdxdna_gem.c
> +++ b/drivers/accel/amdxdna/amdxdna_gem.c
> @@ -552,7 +552,7 @@ int amdxdna_drm_get_bo_info_ioctl(struct drm_device *dev, void *data, struct drm
> struct drm_gem_object *gobj;
> int ret = 0;
>
> - if (args->ext || args->ext_flags)
> + if (args->ext || args->ext_flags || args->pad)
> return -EINVAL;
>
> gobj = drm_gem_object_lookup(filp, args->handle);
> diff --git a/include/uapi/drm/amdxdna_accel.h b/include/uapi/drm/amdxdna_accel.h
> index e4edb52bc27b..a706ead39082 100644
> --- a/include/uapi/drm/amdxdna_accel.h
> +++ b/include/uapi/drm/amdxdna_accel.h
> @@ -87,7 +87,7 @@ struct amdxdna_drm_create_hwctx {
> /**
> * struct amdxdna_drm_destroy_hwctx - Destroy hardware context.
> * @handle: Hardware context handle.
> - * @pad: Structure padding.
> + * @pad: MBZ.
> */
> struct amdxdna_drm_destroy_hwctx {
> __u32 handle;
> @@ -98,7 +98,7 @@ struct amdxdna_drm_destroy_hwctx {
> * struct amdxdna_cu_config - configuration for one CU
> * @cu_bo: CU configuration buffer bo handle.
> * @cu_func: Function of a CU.
> - * @pad: Structure padding.
> + * @pad: MBZ.
> */
> struct amdxdna_cu_config {
> __u32 cu_bo;
> @@ -109,7 +109,7 @@ struct amdxdna_cu_config {
> /**
> * struct amdxdna_hwctx_param_config_cu - configuration for CUs in hardware context
> * @num_cus: Number of CUs to configure.
> - * @pad: Structure padding.
> + * @pad: MBZ.
> * @cu_configs: Array of CU configurations of struct amdxdna_cu_config.
> */
> struct amdxdna_hwctx_param_config_cu {
> @@ -122,7 +122,6 @@ enum amdxdna_drm_config_hwctx_param {
> DRM_AMDXDNA_HWCTX_CONFIG_CU,
> DRM_AMDXDNA_HWCTX_ASSIGN_DBG_BUF,
> DRM_AMDXDNA_HWCTX_REMOVE_DBG_BUF,
> - DRM_AMDXDNA_HWCTX_CONFIG_NUM
This change actually meant here? It seems like it should be it's own
change.
> };
>
> /**
> @@ -133,7 +132,7 @@ enum amdxdna_drm_config_hwctx_param {
> * @param_val: A structure specified by the param_type struct member.
> * @param_val_size: Size of the parameter buffer pointed to by the param_val.
> * If param_val is not a pointer, driver can ignore this.
> - * @pad: Structure padding.
> + * @pad: MBZ.
> *
> * Note: if the param_val is a pointer pointing to a buffer, the maximum size
> * of the buffer is 4KiB(PAGE_SIZE).
> @@ -175,7 +174,7 @@ struct amdxdna_drm_create_bo {
> * @ext: MBZ.
> * @ext_flags: MBZ.
> * @handle: DRM buffer object handle.
> - * @pad: Structure padding.
> + * @pad: MBZ.
> * @map_offset: Returned DRM fake offset for mmap().
> * @vaddr: Returned user VA of buffer. 0 in case user needs mmap().
> * @xdna_addr: Returned XDNA device virtual address.
Powered by blists - more mailing lists