Re: [char-misc-next 1/6] mei: hbm: use sizeof of variable instead of struct type

From: Gustavo A. R. Silva
Date: Thu Jul 23 2020 - 13:43:01 EST

Next message: Lina Iyer: "Re: [PATCH] soc: qcom: rpmh-rsc: Sleep waiting for tcs slots to be free"
Previous message: Andrei Vagin: "Re: [PATCH v5 0/6] arm64: add the time namespace support"
In reply to: Greg Kroah-Hartman: "Re: [char-misc-next 1/6] mei: hbm: use sizeof of variable instead of struct type"
Next in thread: Tomas Winkler: "[char-misc-next 2/6] mei: ioctl: use sizeof of variable instead of struct type"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On 7/23/20 09:59, Tomas Winkler wrote:
> There is a possibility of bug when variable type has changed but
> corresponding struct passed to the sizeof has not.
>
> Cc: Gustavo A. R. Silva <gustavoars@xxxxxxxxxx>
> Signed-off-by: Tomas Winkler <tomas.winkler@xxxxxxxxx>
Reviewed-by: Gustavo A. R. Silva <gustavoars@xxxxxxxxxx>

Thanks
--
Gustavo

> ---
> drivers/misc/mei/hbm.c | 74 ++++++++++++++++++------------------------
> 1 file changed, 32 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/misc/mei/hbm.c b/drivers/misc/mei/hbm.c
> index a44094cdbc36..308caee86920 100644
> --- a/drivers/misc/mei/hbm.c
> +++ b/drivers/misc/mei/hbm.c
> @@ -1,6 +1,6 @@
> // SPDX-License-Identifier: GPL-2.0
> /*
> - * Copyright (c) 2003-2019, Intel Corporation. All rights reserved.
> + * Copyright (c) 2003-2020, Intel Corporation. All rights reserved.
> * Intel Management Engine Interface (Intel MEI) Linux driver
> */
> #include <linux/export.h>
> @@ -257,22 +257,21 @@ int mei_hbm_start_wait(struct mei_device *dev)
> int mei_hbm_start_req(struct mei_device *dev)
> {
> struct mei_msg_hdr mei_hdr;
> - struct hbm_host_version_request start_req;
> - const size_t len = sizeof(struct hbm_host_version_request);
> + struct hbm_host_version_request req;
> int ret;
>
> mei_hbm_reset(dev);
>
> - mei_hbm_hdr(&mei_hdr, len);
> + mei_hbm_hdr(&mei_hdr, sizeof(req));
>
> /* host start message */
> - memset(&start_req, 0, len);
> - start_req.hbm_cmd = HOST_START_REQ_CMD;
> - start_req.host_version.major_version = HBM_MAJOR_VERSION;
> - start_req.host_version.minor_version = HBM_MINOR_VERSION;
> + memset(&req, 0, sizeof(req));
> + req.hbm_cmd = HOST_START_REQ_CMD;
> + req.host_version.major_version = HBM_MAJOR_VERSION;
> + req.host_version.minor_version = HBM_MINOR_VERSION;
>
> dev->hbm_state = MEI_HBM_IDLE;
> - ret = mei_hbm_write_message(dev, &mei_hdr, &start_req);
> + ret = mei_hbm_write_message(dev, &mei_hdr, &req);
> if (ret) {
> dev_err(dev->dev, "version message write failed: ret = %d\n",
> ret);
> @@ -295,13 +294,12 @@ static int mei_hbm_dma_setup_req(struct mei_device *dev)
> {
> struct mei_msg_hdr mei_hdr;
> struct hbm_dma_setup_request req;
> - const size_t len = sizeof(struct hbm_dma_setup_request);
> unsigned int i;
> int ret;
>
> - mei_hbm_hdr(&mei_hdr, len);
> + mei_hbm_hdr(&mei_hdr, sizeof(req));
>
> - memset(&req, 0, len);
> + memset(&req, 0, sizeof(req));
> req.hbm_cmd = MEI_HBM_DMA_SETUP_REQ_CMD;
> for (i = 0; i < DMA_DSCR_NUM; i++) {
> phys_addr_t paddr;
> @@ -337,21 +335,19 @@ static int mei_hbm_dma_setup_req(struct mei_device *dev)
> static int mei_hbm_enum_clients_req(struct mei_device *dev)
> {
> struct mei_msg_hdr mei_hdr;
> - struct hbm_host_enum_request enum_req;
> - const size_t len = sizeof(struct hbm_host_enum_request);
> + struct hbm_host_enum_request req;
> int ret;
>
> /* enumerate clients */
> - mei_hbm_hdr(&mei_hdr, len);
> + mei_hbm_hdr(&mei_hdr, sizeof(req));
>
> - memset(&enum_req, 0, len);
> - enum_req.hbm_cmd = HOST_ENUM_REQ_CMD;
> - enum_req.flags |= dev->hbm_f_dc_supported ?
> - MEI_HBM_ENUM_F_ALLOW_ADD : 0;
> - enum_req.flags |= dev->hbm_f_ie_supported ?
> + memset(&req, 0, sizeof(req));
> + req.hbm_cmd = HOST_ENUM_REQ_CMD;
> + req.flags |= dev->hbm_f_dc_supported ? MEI_HBM_ENUM_F_ALLOW_ADD : 0;
> + req.flags |= dev->hbm_f_ie_supported ?
> MEI_HBM_ENUM_F_IMMEDIATE_ENUM : 0;
>
> - ret = mei_hbm_write_message(dev, &mei_hdr, &enum_req);
> + ret = mei_hbm_write_message(dev, &mei_hdr, &req);
> if (ret) {
> dev_err(dev->dev, "enumeration request write failed: ret = %d.\n",
> ret);
> @@ -380,7 +376,7 @@ static int mei_hbm_me_cl_add(struct mei_device *dev,
>
> mei_me_cl_rm_by_uuid(dev, uuid);
>
> - me_cl = kzalloc(sizeof(struct mei_me_client), GFP_KERNEL);
> + me_cl = kzalloc(sizeof(*me_cl), GFP_KERNEL);
> if (!me_cl)
> return -ENOMEM;
>
> @@ -408,14 +404,13 @@ static int mei_hbm_add_cl_resp(struct mei_device *dev, u8 addr, u8 status)
> {
> struct mei_msg_hdr mei_hdr;
> struct hbm_add_client_response resp;
> - const size_t len = sizeof(struct hbm_add_client_response);
> int ret;
>
> dev_dbg(dev->dev, "adding client response\n");
>
> - mei_hbm_hdr(&mei_hdr, len);
> + mei_hbm_hdr(&mei_hdr, sizeof(resp));
>
> - memset(&resp, 0, sizeof(struct hbm_add_client_response));
> + memset(&resp, 0, sizeof(resp));
> resp.hbm_cmd = MEI_HBM_ADD_CLIENT_RES_CMD;
> resp.me_addr = addr;
> resp.status = status;
> @@ -469,11 +464,10 @@ int mei_hbm_cl_notify_req(struct mei_device *dev,
>
> struct mei_msg_hdr mei_hdr;
> struct hbm_notification_request req;
> - const size_t len = sizeof(struct hbm_notification_request);
> int ret;
>
> - mei_hbm_hdr(&mei_hdr, len);
> - mei_hbm_cl_hdr(cl, MEI_HBM_NOTIFY_REQ_CMD, &req, len);
> + mei_hbm_hdr(&mei_hdr, sizeof(req));
> + mei_hbm_cl_hdr(cl, MEI_HBM_NOTIFY_REQ_CMD, &req, sizeof(req));
>
> req.start = start;
>
> @@ -580,8 +574,7 @@ static void mei_hbm_cl_notify(struct mei_device *dev,
> static int mei_hbm_prop_req(struct mei_device *dev, unsigned long start_idx)
> {
> struct mei_msg_hdr mei_hdr;
> - struct hbm_props_request prop_req;
> - const size_t len = sizeof(struct hbm_props_request);
> + struct hbm_props_request req;
> unsigned long addr;
> int ret;
>
> @@ -591,18 +584,17 @@ static int mei_hbm_prop_req(struct mei_device *dev, unsigned long start_idx)
> if (addr == MEI_CLIENTS_MAX) {
> dev->hbm_state = MEI_HBM_STARTED;
> mei_host_client_init(dev);
> -
> return 0;
> }
>
> - mei_hbm_hdr(&mei_hdr, len);
> + mei_hbm_hdr(&mei_hdr, sizeof(req));
>
> - memset(&prop_req, 0, sizeof(struct hbm_props_request));
> + memset(&req, 0, sizeof(req));
>
> - prop_req.hbm_cmd = HOST_CLIENT_PROPERTIES_REQ_CMD;
> - prop_req.me_addr = addr;
> + req.hbm_cmd = HOST_CLIENT_PROPERTIES_REQ_CMD;
> + req.me_addr = addr;
>
> - ret = mei_hbm_write_message(dev, &mei_hdr, &prop_req);
> + ret = mei_hbm_write_message(dev, &mei_hdr, &req);
> if (ret) {
> dev_err(dev->dev, "properties request write failed: ret = %d\n",
> ret);
> @@ -628,15 +620,14 @@ int mei_hbm_pg(struct mei_device *dev, u8 pg_cmd)
> {
> struct mei_msg_hdr mei_hdr;
> struct hbm_power_gate req;
> - const size_t len = sizeof(struct hbm_power_gate);
> int ret;
>
> if (!dev->hbm_f_pg_supported)
> return -EOPNOTSUPP;
>
> - mei_hbm_hdr(&mei_hdr, len);
> + mei_hbm_hdr(&mei_hdr, sizeof(req));
>
> - memset(&req, 0, len);
> + memset(&req, 0, sizeof(req));
> req.hbm_cmd = pg_cmd;
>
> ret = mei_hbm_write_message(dev, &mei_hdr, &req);
> @@ -657,11 +648,10 @@ static int mei_hbm_stop_req(struct mei_device *dev)
> {
> struct mei_msg_hdr mei_hdr;
> struct hbm_host_stop_request req;
> - const size_t len = sizeof(struct hbm_host_stop_request);
>
> - mei_hbm_hdr(&mei_hdr, len);
> + mei_hbm_hdr(&mei_hdr, sizeof(req));
>
> - memset(&req, 0, len);
> + memset(&req, 0, sizeof(req));
> req.hbm_cmd = HOST_STOP_REQ_CMD;
> req.reason = DRIVER_STOP_REQUEST;
>
>

Next message: Lina Iyer: "Re: [PATCH] soc: qcom: rpmh-rsc: Sleep waiting for tcs slots to be free"
Previous message: Andrei Vagin: "Re: [PATCH v5 0/6] arm64: add the time namespace support"
In reply to: Greg Kroah-Hartman: "Re: [char-misc-next 1/6] mei: hbm: use sizeof of variable instead of struct type"
Next in thread: Tomas Winkler: "[char-misc-next 2/6] mei: ioctl: use sizeof of variable instead of struct type"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]