linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [char-misc-next 0/6] mei: remove one-element arrays
@ 2020-07-23 14:59 Tomas Winkler
  2020-07-23 14:59 ` [char-misc-next 1/6] mei: hbm: use sizeof of variable instead of struct type Tomas Winkler
                   ` (5 more replies)
  0 siblings, 6 replies; 16+ messages in thread
From: Tomas Winkler @ 2020-07-23 14:59 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Alexander Usyskin, linux-kernel, Tomas Winkler,
	Gustavo A. R. Silva, Arnd Bergmann


1. Remove one-element arrays from hw.h
2. Adjust to preferred method of getting size of structure
from memory allocation and movement. sizeof(*var) 
instead of sizeof(struct some_struct)


Tomas Winkler (6):
  mei: hbm: use sizeof of variable instead of struct type
  mei: ioctl: use sizeof of variable instead of struct type
  mei: bus: use sizeof of variable instead of struct type
  mei: client: use sizeof of variable instead of struct type
  mei: hw: use sizeof of variable instead of struct type
  mei: hw: don't use one element arrays

 drivers/misc/mei/bus-fixup.c | 23 ++++++-----
 drivers/misc/mei/bus.c       |  2 +-
 drivers/misc/mei/client.c    |  8 ++--
 drivers/misc/mei/hbm.c       | 74 ++++++++++++++++--------------------
 drivers/misc/mei/hw-me.c     |  5 +--
 drivers/misc/mei/hw-txe.c    |  5 +--
 drivers/misc/mei/hw.h        |  8 ++--
 drivers/misc/mei/main.c      |  6 +--
 8 files changed, 59 insertions(+), 72 deletions(-)

-- 
2.25.4


^ permalink raw reply	[flat|nested] 16+ messages in thread
* [char-misc-next 1/6] mei: hbm: use sizeof of variable instead of struct type
  2020-07-23 14:59 [char-misc-next 0/6] mei: remove one-element arrays Tomas Winkler
@ 2020-07-23 14:59 ` Tomas Winkler
  2020-07-23 15:13   ` Greg Kroah-Hartman
  2020-07-23 17:01   ` Gustavo A. R. Silva
  2020-07-23 14:59 ` [char-misc-next 2/6] mei: ioctl: " Tomas Winkler
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 16+ messages in thread
From: Tomas Winkler @ 2020-07-23 14:59 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Alexander Usyskin, linux-kernel, Tomas Winkler,
	Gustavo A . R . Silva, Arnd Bergmann

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@kernel.org>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 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;
 
-- 
2.25.4


^ permalink raw reply related	[flat|nested] 16+ messages in thread
* [char-misc-next 2/6] mei: ioctl: use sizeof of variable instead of struct type
  2020-07-23 14:59 [char-misc-next 0/6] mei: remove one-element arrays Tomas Winkler
  2020-07-23 14:59 ` [char-misc-next 1/6] mei: hbm: use sizeof of variable instead of struct type Tomas Winkler
@ 2020-07-23 14:59 ` Tomas Winkler
  2020-07-23 16:55   ` Gustavo A. R. Silva
  2020-07-23 14:59 ` [char-misc-next 3/6] mei: bus: " Tomas Winkler
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 16+ messages in thread
From: Tomas Winkler @ 2020-07-23 14:59 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Alexander Usyskin, linux-kernel, Tomas Winkler,
	Gustavo A . R . Silva, Arnd Bergmann

Use sizeof(connect_data))) instead of
sizeof(struct mei_connect_client_data) when copying data
between user space and kernel.

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@kernel.org>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/misc/mei/main.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/misc/mei/main.c b/drivers/misc/mei/main.c
index f17297f2943d..05e6ad6d4d54 100644
--- a/drivers/misc/mei/main.c
+++ b/drivers/misc/mei/main.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 /*
- * Copyright (c) 2003-2018, Intel Corporation. All rights reserved.
+ * Copyright (c) 2003-2020, Intel Corporation. All rights reserved.
  * Intel Management Engine Interface (Intel MEI) Linux driver
  */
 
@@ -476,7 +476,7 @@ static long mei_ioctl(struct file *file, unsigned int cmd, unsigned long data)
 	case IOCTL_MEI_CONNECT_CLIENT:
 		dev_dbg(dev->dev, ": IOCTL_MEI_CONNECT_CLIENT.\n");
 		if (copy_from_user(&connect_data, (char __user *)data,
-				sizeof(struct mei_connect_client_data))) {
+				   sizeof(connect_data))) {
 			dev_dbg(dev->dev, "failed to copy data from userland\n");
 			rets = -EFAULT;
 			goto out;
@@ -488,7 +488,7 @@ static long mei_ioctl(struct file *file, unsigned int cmd, unsigned long data)
 
 		/* if all is ok, copying the data back to user. */
 		if (copy_to_user((char __user *)data, &connect_data,
-				sizeof(struct mei_connect_client_data))) {
+				 sizeof(connect_data))) {
 			dev_dbg(dev->dev, "failed to copy data to userland\n");
 			rets = -EFAULT;
 			goto out;
-- 
2.25.4


^ permalink raw reply related	[flat|nested] 16+ messages in thread
* [char-misc-next 3/6] mei: bus: use sizeof of variable instead of struct type
  2020-07-23 14:59 [char-misc-next 0/6] mei: remove one-element arrays Tomas Winkler
  2020-07-23 14:59 ` [char-misc-next 1/6] mei: hbm: use sizeof of variable instead of struct type Tomas Winkler
  2020-07-23 14:59 ` [char-misc-next 2/6] mei: ioctl: " Tomas Winkler
@ 2020-07-23 14:59 ` Tomas Winkler
  2020-07-23 16:55   ` Gustavo A. R. Silva
  2020-07-23 14:59 ` [char-misc-next 4/6] mei: client: " Tomas Winkler
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 16+ messages in thread
From: Tomas Winkler @ 2020-07-23 14:59 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Alexander Usyskin, linux-kernel, Tomas Winkler,
	Gustavo A . R . Silva, Arnd Bergmann

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@kernel.org>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/misc/mei/bus-fixup.c | 23 +++++++++++------------
 drivers/misc/mei/bus.c       |  2 +-
 2 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/drivers/misc/mei/bus-fixup.c b/drivers/misc/mei/bus-fixup.c
index 910f059b3384..07ba16d46690 100644
--- a/drivers/misc/mei/bus-fixup.c
+++ b/drivers/misc/mei/bus-fixup.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 /*
- * Copyright (c) 2013-2019, Intel Corporation. All rights reserved.
+ * Copyright (c) 2013-2020, Intel Corporation. All rights reserved.
  * Intel Management Engine Interface (Intel MEI) Linux driver
  */
 
@@ -159,17 +159,17 @@ static int mei_osver(struct mei_cl_device *cldev)
 static int mei_fwver(struct mei_cl_device *cldev)
 {
 	char buf[MKHI_FWVER_BUF_LEN];
-	struct mkhi_msg *req;
+	struct mkhi_msg req;
+	struct mkhi_msg *rsp;
 	struct mkhi_fw_ver *fwver;
 	int bytes_recv, ret, i;
 
 	memset(buf, 0, sizeof(buf));
 
-	req = (struct mkhi_msg *)buf;
-	req->hdr.group_id = MKHI_GEN_GROUP_ID;
-	req->hdr.command = MKHI_GEN_GET_FW_VERSION_CMD;
+	req.hdr.group_id = MKHI_GEN_GROUP_ID;
+	req.hdr.command = MKHI_GEN_GET_FW_VERSION_CMD;
 
-	ret = __mei_cl_send(cldev->cl, buf, sizeof(struct mkhi_msg_hdr),
+	ret = __mei_cl_send(cldev->cl, (u8 *)&req, sizeof(req),
 			    MEI_CL_IO_TX_BLOCKING);
 	if (ret < 0) {
 		dev_err(&cldev->dev, "Could not send ReqFWVersion cmd\n");
@@ -188,7 +188,8 @@ static int mei_fwver(struct mei_cl_device *cldev)
 		return -EIO;
 	}
 
-	fwver = (struct mkhi_fw_ver *)req->data;
+	rsp = (struct mkhi_msg *)buf;
+	fwver = (struct mkhi_fw_ver *)rsp->data;
 	memset(cldev->bus->fw_ver, 0, sizeof(cldev->bus->fw_ver));
 	for (i = 0; i < MEI_MAX_FW_VER_BLOCKS; i++) {
 		if ((size_t)bytes_recv < MKHI_FWVER_LEN(i + 1))
@@ -329,16 +330,14 @@ static int mei_nfc_if_version(struct mei_cl *cl,
 
 	WARN_ON(mutex_is_locked(&bus->device_lock));
 
-	ret = __mei_cl_send(cl, (u8 *)&cmd, sizeof(struct mei_nfc_cmd),
-			    MEI_CL_IO_TX_BLOCKING);
+	ret = __mei_cl_send(cl, (u8 *)&cmd, sizeof(cmd), MEI_CL_IO_TX_BLOCKING);
 	if (ret < 0) {
 		dev_err(bus->dev, "Could not send IF version cmd\n");
 		return ret;
 	}
 
 	/* to be sure on the stack we alloc memory */
-	if_version_length = sizeof(struct mei_nfc_reply) +
-		sizeof(struct mei_nfc_if_version);
+	if_version_length = sizeof(*reply) + sizeof(*ver);
 
 	reply = kzalloc(if_version_length, GFP_KERNEL);
 	if (!reply)
@@ -352,7 +351,7 @@ static int mei_nfc_if_version(struct mei_cl *cl,
 		goto err;
 	}
 
-	memcpy(ver, reply->data, sizeof(struct mei_nfc_if_version));
+	memcpy(ver, reply->data, sizeof(*ver));
 
 	dev_info(bus->dev, "NFC MEI VERSION: IVN 0x%x Vendor ID 0x%x Type 0x%x\n",
 		ver->fw_ivn, ver->vendor_id, ver->radio_type);
diff --git a/drivers/misc/mei/bus.c b/drivers/misc/mei/bus.c
index f476dbc7252b..a6dfc3ce1db2 100644
--- a/drivers/misc/mei/bus.c
+++ b/drivers/misc/mei/bus.c
@@ -931,7 +931,7 @@ static struct mei_cl_device *mei_cl_bus_dev_alloc(struct mei_device *bus,
 	struct mei_cl_device *cldev;
 	struct mei_cl *cl;
 
-	cldev = kzalloc(sizeof(struct mei_cl_device), GFP_KERNEL);
+	cldev = kzalloc(sizeof(*cldev), GFP_KERNEL);
 	if (!cldev)
 		return NULL;
 
-- 
2.25.4


^ permalink raw reply related	[flat|nested] 16+ messages in thread
* [char-misc-next 4/6] mei: client: use sizeof of variable instead of struct type
  2020-07-23 14:59 [char-misc-next 0/6] mei: remove one-element arrays Tomas Winkler
                   ` (2 preceding siblings ...)
  2020-07-23 14:59 ` [char-misc-next 3/6] mei: bus: " Tomas Winkler
@ 2020-07-23 14:59 ` Tomas Winkler
  2020-07-23 16:54   ` Gustavo A. R. Silva
  2020-07-23 14:59 ` [char-misc-next 5/6] mei: hw: " Tomas Winkler
  2020-07-23 14:59 ` [char-misc-next 6/6] mei: hw: don't use one element arrays Tomas Winkler
  5 siblings, 1 reply; 16+ messages in thread
From: Tomas Winkler @ 2020-07-23 14:59 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Alexander Usyskin, linux-kernel, Tomas Winkler,
	Gustavo A . R . Silva, Arnd Bergmann

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@kernel.org>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/misc/mei/client.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/misc/mei/client.c b/drivers/misc/mei/client.c
index b32c825a0945..2572887d99b6 100644
--- a/drivers/misc/mei/client.c
+++ b/drivers/misc/mei/client.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
  */
 
@@ -369,7 +369,7 @@ static struct mei_cl_cb *mei_io_cb_init(struct mei_cl *cl,
 {
 	struct mei_cl_cb *cb;
 
-	cb = kzalloc(sizeof(struct mei_cl_cb), GFP_KERNEL);
+	cb = kzalloc(sizeof(*cb), GFP_KERNEL);
 	if (!cb)
 		return NULL;
 
@@ -552,7 +552,7 @@ int mei_cl_flush_queues(struct mei_cl *cl, const struct file *fp)
  */
 static void mei_cl_init(struct mei_cl *cl, struct mei_device *dev)
 {
-	memset(cl, 0, sizeof(struct mei_cl));
+	memset(cl, 0, sizeof(*cl));
 	init_waitqueue_head(&cl->wait);
 	init_waitqueue_head(&cl->rx_wait);
 	init_waitqueue_head(&cl->tx_wait);
@@ -575,7 +575,7 @@ struct mei_cl *mei_cl_allocate(struct mei_device *dev)
 {
 	struct mei_cl *cl;
 
-	cl = kmalloc(sizeof(struct mei_cl), GFP_KERNEL);
+	cl = kmalloc(sizeof(*cl), GFP_KERNEL);
 	if (!cl)
 		return NULL;
 
-- 
2.25.4


^ permalink raw reply related	[flat|nested] 16+ messages in thread
* [char-misc-next 5/6] mei: hw: use sizeof of variable instead of struct type
  2020-07-23 14:59 [char-misc-next 0/6] mei: remove one-element arrays Tomas Winkler
                   ` (3 preceding siblings ...)
  2020-07-23 14:59 ` [char-misc-next 4/6] mei: client: " Tomas Winkler
@ 2020-07-23 14:59 ` Tomas Winkler
  2020-07-23 16:53   ` Gustavo A. R. Silva
  2020-07-23 14:59 ` [char-misc-next 6/6] mei: hw: don't use one element arrays Tomas Winkler
  5 siblings, 1 reply; 16+ messages in thread
From: Tomas Winkler @ 2020-07-23 14:59 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Alexander Usyskin, linux-kernel, Tomas Winkler,
	Gustavo A . R . Silva, Arnd Bergmann

Use sizeof(*dev) + sizeof(*hw) instead of
sizeof(struct mei_device) + sizeof(struct mei_me_hw)

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@kernel.org>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/misc/mei/hw-me.c  | 5 ++---
 drivers/misc/mei/hw-txe.c | 5 ++---
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/misc/mei/hw-me.c b/drivers/misc/mei/hw-me.c
index c51d3da8f333..7692b69abcb5 100644
--- a/drivers/misc/mei/hw-me.c
+++ b/drivers/misc/mei/hw-me.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
  */
 
@@ -1600,8 +1600,7 @@ struct mei_device *mei_me_dev_init(struct device *parent,
 	struct mei_me_hw *hw;
 	int i;
 
-	dev = devm_kzalloc(parent, sizeof(struct mei_device) +
-			   sizeof(struct mei_me_hw), GFP_KERNEL);
+	dev = devm_kzalloc(parent, sizeof(*dev) + sizeof(*hw), GFP_KERNEL);
 	if (!dev)
 		return NULL;
 
diff --git a/drivers/misc/mei/hw-txe.c b/drivers/misc/mei/hw-txe.c
index 785b260b3ae9..a4e854b9b9e6 100644
--- a/drivers/misc/mei/hw-txe.c
+++ b/drivers/misc/mei/hw-txe.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 /*
- * Copyright (c) 2013-2019, Intel Corporation. All rights reserved.
+ * Copyright (c) 2013-2020, Intel Corporation. All rights reserved.
  * Intel Management Engine Interface (Intel MEI) Linux driver
  */
 
@@ -1201,8 +1201,7 @@ struct mei_device *mei_txe_dev_init(struct pci_dev *pdev)
 	struct mei_device *dev;
 	struct mei_txe_hw *hw;
 
-	dev = devm_kzalloc(&pdev->dev, sizeof(struct mei_device) +
-			   sizeof(struct mei_txe_hw), GFP_KERNEL);
+	dev = devm_kzalloc(&pdev->dev, sizeof(*dev) + sizeof(*hw), GFP_KERNEL);
 	if (!dev)
 		return NULL;
 
-- 
2.25.4


^ permalink raw reply related	[flat|nested] 16+ messages in thread
* [char-misc-next 6/6] mei: hw: don't use one element arrays
  2020-07-23 14:59 [char-misc-next 0/6] mei: remove one-element arrays Tomas Winkler
                   ` (4 preceding siblings ...)
  2020-07-23 14:59 ` [char-misc-next 5/6] mei: hw: " Tomas Winkler
@ 2020-07-23 14:59 ` Tomas Winkler
  2020-07-23 16:49   ` Gustavo A. R. Silva
  5 siblings, 1 reply; 16+ messages in thread
From: Tomas Winkler @ 2020-07-23 14:59 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Alexander Usyskin, linux-kernel, Tomas Winkler,
	Gustavo A . R . Silva, Arnd Bergmann

Replace the single element arrays with a simple value type u8 reserved,
even thought is is not used for dynamically sized trailing elements
it confuses the effort of replacing one-element arrays with
flexible arrays for that purpose.

Link: https://github.com/KSPP/linux/issues/79
Cc: Gustavo A. R. Silva <gustavoars@kernel.org>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/misc/mei/hw.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/misc/mei/hw.h b/drivers/misc/mei/hw.h
index b1a8d5ec88b3..26fa92cb7f7a 100644
--- a/drivers/misc/mei/hw.h
+++ b/drivers/misc/mei/hw.h
@@ -1,6 +1,6 @@
 /* SPDX-License-Identifier: GPL-2.0 */
 /*
- * Copyright (c) 2003-2018, Intel Corporation. All rights reserved
+ * Copyright (c) 2003-2020, Intel Corporation. All rights reserved
  * Intel Management Engine Interface (Intel MEI) Linux driver
  */
 
@@ -319,7 +319,7 @@ struct hbm_props_response {
 	u8 hbm_cmd;
 	u8 me_addr;
 	u8 status;
-	u8 reserved[1];
+	u8 reserved;
 	struct mei_client_properties client_properties;
 } __packed;
 
@@ -352,7 +352,7 @@ struct hbm_add_client_response {
 	u8 hbm_cmd;
 	u8 me_addr;
 	u8 status;
-	u8 reserved[1];
+	u8 reserved;
 } __packed;
 
 /**
@@ -461,7 +461,7 @@ struct hbm_notification {
 	u8 hbm_cmd;
 	u8 me_addr;
 	u8 host_addr;
-	u8 reserved[1];
+	u8 reserved;
 } __packed;
 
 /**
-- 
2.25.4


^ permalink raw reply related	[flat|nested] 16+ messages in thread
* Re: [char-misc-next 1/6] mei: hbm: use sizeof of variable instead of struct type
  2020-07-23 14:59 ` [char-misc-next 1/6] mei: hbm: use sizeof of variable instead of struct type Tomas Winkler
@ 2020-07-23 15:13   ` Greg Kroah-Hartman
  2020-07-23 17:04     ` Gustavo A. R. Silva
  2020-07-23 17:01   ` Gustavo A. R. Silva
  1 sibling, 1 reply; 16+ messages in thread
From: Greg Kroah-Hartman @ 2020-07-23 15:13 UTC (permalink / raw)
  To: Tomas Winkler
  Cc: Alexander Usyskin, linux-kernel, Gustavo A . R . Silva, Arnd Bergmann

On Thu, Jul 23, 2020 at 05:59:22PM +0300, 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@kernel.org>
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> ---
>  drivers/misc/mei/hbm.c | 74 ++++++++++++++++++------------------------
>  1 file changed, 32 insertions(+), 42 deletions(-)

This doesn't apply to my tree as I've applied Gustavo's patch.  Should I
revert that first?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [char-misc-next 6/6] mei: hw: don't use one element arrays
  2020-07-23 14:59 ` [char-misc-next 6/6] mei: hw: don't use one element arrays Tomas Winkler
@ 2020-07-23 16:49   ` Gustavo A. R. Silva
  0 siblings, 0 replies; 16+ messages in thread
From: Gustavo A. R. Silva @ 2020-07-23 16:49 UTC (permalink / raw)
  To: Tomas Winkler, Greg Kroah-Hartman
  Cc: Alexander Usyskin, linux-kernel, Gustavo A . R . Silva, Arnd Bergmann



On 7/23/20 09:59, Tomas Winkler wrote:
> Replace the single element arrays with a simple value type u8 reserved,
> even thought is is not used for dynamically sized trailing elements
> it confuses the effort of replacing one-element arrays with
> flexible arrays for that purpose.
> 
> Link: https://github.com/KSPP/linux/issues/79
> Cc: Gustavo A. R. Silva <gustavoars@kernel.org>
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>

Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org>

Thanks
--
Gustavo

> ---
>  drivers/misc/mei/hw.h | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/misc/mei/hw.h b/drivers/misc/mei/hw.h
> index b1a8d5ec88b3..26fa92cb7f7a 100644
> --- a/drivers/misc/mei/hw.h
> +++ b/drivers/misc/mei/hw.h
> @@ -1,6 +1,6 @@
>  /* SPDX-License-Identifier: GPL-2.0 */
>  /*
> - * Copyright (c) 2003-2018, Intel Corporation. All rights reserved
> + * Copyright (c) 2003-2020, Intel Corporation. All rights reserved
>   * Intel Management Engine Interface (Intel MEI) Linux driver
>   */
>  
> @@ -319,7 +319,7 @@ struct hbm_props_response {
>  	u8 hbm_cmd;
>  	u8 me_addr;
>  	u8 status;
> -	u8 reserved[1];
> +	u8 reserved;
>  	struct mei_client_properties client_properties;
>  } __packed;
>  
> @@ -352,7 +352,7 @@ struct hbm_add_client_response {
>  	u8 hbm_cmd;
>  	u8 me_addr;
>  	u8 status;
> -	u8 reserved[1];
> +	u8 reserved;
>  } __packed;
>  
>  /**
> @@ -461,7 +461,7 @@ struct hbm_notification {
>  	u8 hbm_cmd;
>  	u8 me_addr;
>  	u8 host_addr;
> -	u8 reserved[1];
> +	u8 reserved;
>  } __packed;
>  
>  /**
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [char-misc-next 5/6] mei: hw: use sizeof of variable instead of struct type
  2020-07-23 14:59 ` [char-misc-next 5/6] mei: hw: " Tomas Winkler
@ 2020-07-23 16:53   ` Gustavo A. R. Silva
  0 siblings, 0 replies; 16+ messages in thread
From: Gustavo A. R. Silva @ 2020-07-23 16:53 UTC (permalink / raw)
  To: Tomas Winkler, Greg Kroah-Hartman
  Cc: Alexander Usyskin, linux-kernel, Gustavo A . R . Silva, Arnd Bergmann



On 7/23/20 09:59, Tomas Winkler wrote:
> Use sizeof(*dev) + sizeof(*hw) instead of
> sizeof(struct mei_device) + sizeof(struct mei_me_hw)
> 
> 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@kernel.org>
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>

Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org>

Thanks
--
Gustavo

> ---
>  drivers/misc/mei/hw-me.c  | 5 ++---
>  drivers/misc/mei/hw-txe.c | 5 ++---
>  2 files changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/misc/mei/hw-me.c b/drivers/misc/mei/hw-me.c
> index c51d3da8f333..7692b69abcb5 100644
> --- a/drivers/misc/mei/hw-me.c
> +++ b/drivers/misc/mei/hw-me.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
>   */
>  
> @@ -1600,8 +1600,7 @@ struct mei_device *mei_me_dev_init(struct device *parent,
>  	struct mei_me_hw *hw;
>  	int i;
>  
> -	dev = devm_kzalloc(parent, sizeof(struct mei_device) +
> -			   sizeof(struct mei_me_hw), GFP_KERNEL);
> +	dev = devm_kzalloc(parent, sizeof(*dev) + sizeof(*hw), GFP_KERNEL);
>  	if (!dev)
>  		return NULL;
>  
> diff --git a/drivers/misc/mei/hw-txe.c b/drivers/misc/mei/hw-txe.c
> index 785b260b3ae9..a4e854b9b9e6 100644
> --- a/drivers/misc/mei/hw-txe.c
> +++ b/drivers/misc/mei/hw-txe.c
> @@ -1,6 +1,6 @@
>  // SPDX-License-Identifier: GPL-2.0
>  /*
> - * Copyright (c) 2013-2019, Intel Corporation. All rights reserved.
> + * Copyright (c) 2013-2020, Intel Corporation. All rights reserved.
>   * Intel Management Engine Interface (Intel MEI) Linux driver
>   */
>  
> @@ -1201,8 +1201,7 @@ struct mei_device *mei_txe_dev_init(struct pci_dev *pdev)
>  	struct mei_device *dev;
>  	struct mei_txe_hw *hw;
>  
> -	dev = devm_kzalloc(&pdev->dev, sizeof(struct mei_device) +
> -			   sizeof(struct mei_txe_hw), GFP_KERNEL);
> +	dev = devm_kzalloc(&pdev->dev, sizeof(*dev) + sizeof(*hw), GFP_KERNEL);
>  	if (!dev)
>  		return NULL;
>  
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [char-misc-next 4/6] mei: client: use sizeof of variable instead of struct type
  2020-07-23 14:59 ` [char-misc-next 4/6] mei: client: " Tomas Winkler
@ 2020-07-23 16:54   ` Gustavo A. R. Silva
  0 siblings, 0 replies; 16+ messages in thread
From: Gustavo A. R. Silva @ 2020-07-23 16:54 UTC (permalink / raw)
  To: Tomas Winkler, Greg Kroah-Hartman
  Cc: Alexander Usyskin, linux-kernel, Gustavo A . R . Silva, Arnd Bergmann



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@kernel.org>
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>

Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org>

Thanks
--
Gustavo

> ---
>  drivers/misc/mei/client.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/misc/mei/client.c b/drivers/misc/mei/client.c
> index b32c825a0945..2572887d99b6 100644
> --- a/drivers/misc/mei/client.c
> +++ b/drivers/misc/mei/client.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
>   */
>  
> @@ -369,7 +369,7 @@ static struct mei_cl_cb *mei_io_cb_init(struct mei_cl *cl,
>  {
>  	struct mei_cl_cb *cb;
>  
> -	cb = kzalloc(sizeof(struct mei_cl_cb), GFP_KERNEL);
> +	cb = kzalloc(sizeof(*cb), GFP_KERNEL);
>  	if (!cb)
>  		return NULL;
>  
> @@ -552,7 +552,7 @@ int mei_cl_flush_queues(struct mei_cl *cl, const struct file *fp)
>   */
>  static void mei_cl_init(struct mei_cl *cl, struct mei_device *dev)
>  {
> -	memset(cl, 0, sizeof(struct mei_cl));
> +	memset(cl, 0, sizeof(*cl));
>  	init_waitqueue_head(&cl->wait);
>  	init_waitqueue_head(&cl->rx_wait);
>  	init_waitqueue_head(&cl->tx_wait);
> @@ -575,7 +575,7 @@ struct mei_cl *mei_cl_allocate(struct mei_device *dev)
>  {
>  	struct mei_cl *cl;
>  
> -	cl = kmalloc(sizeof(struct mei_cl), GFP_KERNEL);
> +	cl = kmalloc(sizeof(*cl), GFP_KERNEL);
>  	if (!cl)
>  		return NULL;
>  
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [char-misc-next 3/6] mei: bus: use sizeof of variable instead of struct type
  2020-07-23 14:59 ` [char-misc-next 3/6] mei: bus: " Tomas Winkler
@ 2020-07-23 16:55   ` Gustavo A. R. Silva
  0 siblings, 0 replies; 16+ messages in thread
From: Gustavo A. R. Silva @ 2020-07-23 16:55 UTC (permalink / raw)
  To: Tomas Winkler, Greg Kroah-Hartman
  Cc: Alexander Usyskin, linux-kernel, Gustavo A . R . Silva, Arnd Bergmann



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@kernel.org>
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>

Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org>

Thanks
--
Gustavo

> ---
>  drivers/misc/mei/bus-fixup.c | 23 +++++++++++------------
>  drivers/misc/mei/bus.c       |  2 +-
>  2 files changed, 12 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/misc/mei/bus-fixup.c b/drivers/misc/mei/bus-fixup.c
> index 910f059b3384..07ba16d46690 100644
> --- a/drivers/misc/mei/bus-fixup.c
> +++ b/drivers/misc/mei/bus-fixup.c
> @@ -1,6 +1,6 @@
>  // SPDX-License-Identifier: GPL-2.0
>  /*
> - * Copyright (c) 2013-2019, Intel Corporation. All rights reserved.
> + * Copyright (c) 2013-2020, Intel Corporation. All rights reserved.
>   * Intel Management Engine Interface (Intel MEI) Linux driver
>   */
>  
> @@ -159,17 +159,17 @@ static int mei_osver(struct mei_cl_device *cldev)
>  static int mei_fwver(struct mei_cl_device *cldev)
>  {
>  	char buf[MKHI_FWVER_BUF_LEN];
> -	struct mkhi_msg *req;
> +	struct mkhi_msg req;
> +	struct mkhi_msg *rsp;
>  	struct mkhi_fw_ver *fwver;
>  	int bytes_recv, ret, i;
>  
>  	memset(buf, 0, sizeof(buf));
>  
> -	req = (struct mkhi_msg *)buf;
> -	req->hdr.group_id = MKHI_GEN_GROUP_ID;
> -	req->hdr.command = MKHI_GEN_GET_FW_VERSION_CMD;
> +	req.hdr.group_id = MKHI_GEN_GROUP_ID;
> +	req.hdr.command = MKHI_GEN_GET_FW_VERSION_CMD;
>  
> -	ret = __mei_cl_send(cldev->cl, buf, sizeof(struct mkhi_msg_hdr),
> +	ret = __mei_cl_send(cldev->cl, (u8 *)&req, sizeof(req),
>  			    MEI_CL_IO_TX_BLOCKING);
>  	if (ret < 0) {
>  		dev_err(&cldev->dev, "Could not send ReqFWVersion cmd\n");
> @@ -188,7 +188,8 @@ static int mei_fwver(struct mei_cl_device *cldev)
>  		return -EIO;
>  	}
>  
> -	fwver = (struct mkhi_fw_ver *)req->data;
> +	rsp = (struct mkhi_msg *)buf;
> +	fwver = (struct mkhi_fw_ver *)rsp->data;
>  	memset(cldev->bus->fw_ver, 0, sizeof(cldev->bus->fw_ver));
>  	for (i = 0; i < MEI_MAX_FW_VER_BLOCKS; i++) {
>  		if ((size_t)bytes_recv < MKHI_FWVER_LEN(i + 1))
> @@ -329,16 +330,14 @@ static int mei_nfc_if_version(struct mei_cl *cl,
>  
>  	WARN_ON(mutex_is_locked(&bus->device_lock));
>  
> -	ret = __mei_cl_send(cl, (u8 *)&cmd, sizeof(struct mei_nfc_cmd),
> -			    MEI_CL_IO_TX_BLOCKING);
> +	ret = __mei_cl_send(cl, (u8 *)&cmd, sizeof(cmd), MEI_CL_IO_TX_BLOCKING);
>  	if (ret < 0) {
>  		dev_err(bus->dev, "Could not send IF version cmd\n");
>  		return ret;
>  	}
>  
>  	/* to be sure on the stack we alloc memory */
> -	if_version_length = sizeof(struct mei_nfc_reply) +
> -		sizeof(struct mei_nfc_if_version);
> +	if_version_length = sizeof(*reply) + sizeof(*ver);
>  
>  	reply = kzalloc(if_version_length, GFP_KERNEL);
>  	if (!reply)
> @@ -352,7 +351,7 @@ static int mei_nfc_if_version(struct mei_cl *cl,
>  		goto err;
>  	}
>  
> -	memcpy(ver, reply->data, sizeof(struct mei_nfc_if_version));
> +	memcpy(ver, reply->data, sizeof(*ver));
>  
>  	dev_info(bus->dev, "NFC MEI VERSION: IVN 0x%x Vendor ID 0x%x Type 0x%x\n",
>  		ver->fw_ivn, ver->vendor_id, ver->radio_type);
> diff --git a/drivers/misc/mei/bus.c b/drivers/misc/mei/bus.c
> index f476dbc7252b..a6dfc3ce1db2 100644
> --- a/drivers/misc/mei/bus.c
> +++ b/drivers/misc/mei/bus.c
> @@ -931,7 +931,7 @@ static struct mei_cl_device *mei_cl_bus_dev_alloc(struct mei_device *bus,
>  	struct mei_cl_device *cldev;
>  	struct mei_cl *cl;
>  
> -	cldev = kzalloc(sizeof(struct mei_cl_device), GFP_KERNEL);
> +	cldev = kzalloc(sizeof(*cldev), GFP_KERNEL);
>  	if (!cldev)
>  		return NULL;
>  
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [char-misc-next 2/6] mei: ioctl: use sizeof of variable instead of struct type
  2020-07-23 14:59 ` [char-misc-next 2/6] mei: ioctl: " Tomas Winkler
@ 2020-07-23 16:55   ` Gustavo A. R. Silva
  0 siblings, 0 replies; 16+ messages in thread
From: Gustavo A. R. Silva @ 2020-07-23 16:55 UTC (permalink / raw)
  To: Tomas Winkler, Greg Kroah-Hartman
  Cc: Alexander Usyskin, linux-kernel, Gustavo A . R . Silva, Arnd Bergmann



On 7/23/20 09:59, Tomas Winkler wrote:
> Use sizeof(connect_data))) instead of
> sizeof(struct mei_connect_client_data) when copying data
> between user space and kernel.
> 
> 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@kernel.org>
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>

Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org>

Thanks
--
Gustavo

> ---
>  drivers/misc/mei/main.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/misc/mei/main.c b/drivers/misc/mei/main.c
> index f17297f2943d..05e6ad6d4d54 100644
> --- a/drivers/misc/mei/main.c
> +++ b/drivers/misc/mei/main.c
> @@ -1,6 +1,6 @@
>  // SPDX-License-Identifier: GPL-2.0
>  /*
> - * Copyright (c) 2003-2018, Intel Corporation. All rights reserved.
> + * Copyright (c) 2003-2020, Intel Corporation. All rights reserved.
>   * Intel Management Engine Interface (Intel MEI) Linux driver
>   */
>  
> @@ -476,7 +476,7 @@ static long mei_ioctl(struct file *file, unsigned int cmd, unsigned long data)
>  	case IOCTL_MEI_CONNECT_CLIENT:
>  		dev_dbg(dev->dev, ": IOCTL_MEI_CONNECT_CLIENT.\n");
>  		if (copy_from_user(&connect_data, (char __user *)data,
> -				sizeof(struct mei_connect_client_data))) {
> +				   sizeof(connect_data))) {
>  			dev_dbg(dev->dev, "failed to copy data from userland\n");
>  			rets = -EFAULT;
>  			goto out;
> @@ -488,7 +488,7 @@ static long mei_ioctl(struct file *file, unsigned int cmd, unsigned long data)
>  
>  		/* if all is ok, copying the data back to user. */
>  		if (copy_to_user((char __user *)data, &connect_data,
> -				sizeof(struct mei_connect_client_data))) {
> +				 sizeof(connect_data))) {
>  			dev_dbg(dev->dev, "failed to copy data to userland\n");
>  			rets = -EFAULT;
>  			goto out;
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [char-misc-next 1/6] mei: hbm: use sizeof of variable instead of struct type
  2020-07-23 14:59 ` [char-misc-next 1/6] mei: hbm: use sizeof of variable instead of struct type Tomas Winkler
  2020-07-23 15:13   ` Greg Kroah-Hartman
@ 2020-07-23 17:01   ` Gustavo A. R. Silva
  1 sibling, 0 replies; 16+ messages in thread
From: Gustavo A. R. Silva @ 2020-07-23 17:01 UTC (permalink / raw)
  To: Tomas Winkler, Greg Kroah-Hartman
  Cc: Alexander Usyskin, linux-kernel, Gustavo A . R . Silva, Arnd Bergmann



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@kernel.org>
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org>

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;
>  
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [char-misc-next 1/6] mei: hbm: use sizeof of variable instead of struct type
  2020-07-23 15:13   ` Greg Kroah-Hartman
@ 2020-07-23 17:04     ` Gustavo A. R. Silva
  2020-07-23 17:29       ` Greg Kroah-Hartman
  0 siblings, 1 reply; 16+ messages in thread
From: Gustavo A. R. Silva @ 2020-07-23 17:04 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Tomas Winkler
  Cc: Alexander Usyskin, linux-kernel, Gustavo A . R . Silva, Arnd Bergmann



On 7/23/20 10:13, Greg Kroah-Hartman wrote:
> On Thu, Jul 23, 2020 at 05:59:22PM +0300, 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@kernel.org>
>> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
>> ---
>>  drivers/misc/mei/hbm.c | 74 ++++++++++++++++++------------------------
>>  1 file changed, 32 insertions(+), 42 deletions(-)
> 
> This doesn't apply to my tree as I've applied Gustavo's patch.  Should I
> revert that first?
> 

Yep; this series doesn't take into account my patch. I'm OK with
reverting it, so we can apply this series.

Thanks
--
Gustavo

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [char-misc-next 1/6] mei: hbm: use sizeof of variable instead of struct type
  2020-07-23 17:04     ` Gustavo A. R. Silva
@ 2020-07-23 17:29       ` Greg Kroah-Hartman
  0 siblings, 0 replies; 16+ messages in thread
From: Greg Kroah-Hartman @ 2020-07-23 17:29 UTC (permalink / raw)
  To: Gustavo A. R. Silva
  Cc: Tomas Winkler, Alexander Usyskin, linux-kernel,
	Gustavo A . R . Silva, Arnd Bergmann

On Thu, Jul 23, 2020 at 12:04:06PM -0500, Gustavo A. R. Silva wrote:
> 
> 
> On 7/23/20 10:13, Greg Kroah-Hartman wrote:
> > On Thu, Jul 23, 2020 at 05:59:22PM +0300, 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@kernel.org>
> >> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> >> ---
> >>  drivers/misc/mei/hbm.c | 74 ++++++++++++++++++------------------------
> >>  1 file changed, 32 insertions(+), 42 deletions(-)
> > 
> > This doesn't apply to my tree as I've applied Gustavo's patch.  Should I
> > revert that first?
> > 
> 
> Yep; this series doesn't take into account my patch. I'm OK with
> reverting it, so we can apply this series.

Ok, will do, thanks!

greg k-h

^ permalink raw reply	[flat|nested] 16+ messages in thread
end of thread, other threads:[~2020-07-23 17:43 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-23 14:59 [char-misc-next 0/6] mei: remove one-element arrays Tomas Winkler
2020-07-23 14:59 ` [char-misc-next 1/6] mei: hbm: use sizeof of variable instead of struct type Tomas Winkler
2020-07-23 15:13   ` Greg Kroah-Hartman
2020-07-23 17:04     ` Gustavo A. R. Silva
2020-07-23 17:29       ` Greg Kroah-Hartman
2020-07-23 17:01   ` Gustavo A. R. Silva
2020-07-23 14:59 ` [char-misc-next 2/6] mei: ioctl: " Tomas Winkler
2020-07-23 16:55   ` Gustavo A. R. Silva
2020-07-23 14:59 ` [char-misc-next 3/6] mei: bus: " Tomas Winkler
2020-07-23 16:55   ` Gustavo A. R. Silva
2020-07-23 14:59 ` [char-misc-next 4/6] mei: client: " Tomas Winkler
2020-07-23 16:54   ` Gustavo A. R. Silva
2020-07-23 14:59 ` [char-misc-next 5/6] mei: hw: " Tomas Winkler
2020-07-23 16:53   ` Gustavo A. R. Silva
2020-07-23 14:59 ` [char-misc-next 6/6] mei: hw: don't use one element arrays Tomas Winkler
2020-07-23 16:49   ` Gustavo A. R. Silva

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).