i40e: fix memcpy with swapped arguments

Submitted by Colin King on March 20, 2017, 2:46 p.m.

Details

Message ID 20170320144627.11524-1-colin.king@canonical.com
State Awaiting Upstream
Delegated to: David Miller
Headers show

Commit Message

Colin King March 20, 2017, 2:46 p.m.
From: Colin Ian King <colin.king@canonical.com>

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 <colin.king@canonical.com>
---
 drivers/net/ethernet/intel/i40e/i40e_client.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Keller, Jacob E March 20, 2017, 11:33 p.m.
> -----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 <jeffrey.t.kirsher@intel.com>; 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 <colin.king@canonical.com>


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

Patch hide | download patch | download mbox

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