[PATCH] i40e: fix memcpy with swapped arguments

Subsystems: intel ethernet drivers, networking drivers, the rest

STALE3428d

2 messages, 2 authors, 2017-03-20 · open the first message on its own page

[PATCH] i40e: fix memcpy with swapped arguments

From: Colin King <hidden>
Date: 2017-03-20 14:47:54

From: Colin Ian King <redacted>

The current code copies an uninitialized params into
cdev->lan_info.params and then passes the uninitialized params
to the call cdev->client->ops->l2_param_change.  I believe the
order of the source and destination in the memcpy is the wrong
way around and should be swapped.

Detected with static analysis by cppcheck

Fixes: 0ef2d5afb12d ("i40e: KISS the client interface")
Signed-off-by: Colin Ian King <redacted>
---
 drivers/net/ethernet/intel/i40e/i40e_client.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_client.c b/drivers/net/ethernet/intel/i40e/i40e_client.c
index a9f0d22a7cf4..191580ed946d 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_client.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_client.c
@@ -147,7 +147,7 @@ void i40e_notify_client_of_l2_param_changes(struct i40e_vsi *vsi)
 		dev_dbg(&vsi->back->pdev->dev, "Client is not open, abort l2 param change\n");
 		return;
 	}
-	memcpy(&cdev->lan_info.params, &params, sizeof(struct i40e_params));
+	memcpy(&params, &cdev->lan_info.params, sizeof(struct i40e_params));
 	cdev->client->ops->l2_param_change(&cdev->lan_info, cdev->client,
 					   &params);
 }
-- 
2.11.0

RE: [Intel-wired-lan] [PATCH] i40e: fix memcpy with swapped arguments

From: "Keller, Jacob E" <jacob.e.keller@intel.com>
Date: 2017-03-20 23:34:17

-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces@lists.osuosl.org] On
Behalf Of Colin King
Sent: Monday, March 20, 2017 7:46 AM
To: Kirsher, Jeffrey T <redacted>; intel-wired-
lan@lists.osuosl.org; netdev@vger.kernel.org
Cc: kernel-janitors@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: [Intel-wired-lan] [PATCH] i40e: fix memcpy with swapped arguments

From: Colin Ian King <redacted>
Hi there,
The current code copies an uninitialized params into
cdev->lan_info.params and then passes the uninitialized params
to the call cdev->client->ops->l2_param_change.  I believe the
order of the source and destination in the memcpy is the wrong
way around and should be swapped.
So you are correct that params is uninitialized. However, the fix here is not correct. Somehow we dropped the code for initializing the parameters.

See commit d7ce6422d6e6 ("i40e: don't check params until after checking for client instance", 2017-02-09) It looks like the commit itself was malformed when applied upstream, and a later commit which should have preserved the changes 3140aa9a78c9 ("i40e: KISS the client interface", 2017-03-14) accidentally dropped them.

I'll provide a patch to get this back into the correct state.

Thanks for catching this.

Regards,
Jake
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help