| 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: <cf132e81-524b-3b13-aa18-4f542f7ca480@amd.com>
Date: Mon, 16 Dec 2024 13:03:53 -0800
From: Lizhi Hou <lizhi.hou@....com>
To: Mario Limonciello <mario.limonciello@....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/16/24 12:38, Mario Limonciello wrote:
> 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>
Sure. Will add it.
>
>> 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.
Agree. Hopefully, the patch 1 - 4 are good to go and I do not need to
resend them. :)
Thanks,
Lizhi
>
>> };
>> /**
>> @@ -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