All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.