| Message ID | 20201020232812.46498-1-ljp@linux.ibm.com |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net] ibmvnic: no need to update adapter->mac_addr before it completes | expand |
| Context | Check | Description |
|---|---|---|
| jkicinski/cover_letter | success | Link |
| jkicinski/fixes_present | success | Link |
| jkicinski/patch_count | success | Link |
| jkicinski/tree_selection | success | Clearly marked for net |
| jkicinski/subject_prefix | success | Link |
| jkicinski/source_inline | success | Was 0 now: 0 |
| jkicinski/verify_signedoff | success | Link |
| jkicinski/module_param | success | Was 0 now: 0 |
| jkicinski/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| jkicinski/kdoc | success | Errors and warnings before: 18 this patch: 18 |
| jkicinski/verify_fixes | success | Link |
| jkicinski/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 14 lines checked |
| jkicinski/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| jkicinski/header_inline | success | Link |
| jkicinski/stable | success | Stable not CCed |
On Tue, 20 Oct 2020 18:28:12 -0500 Lijun Pan wrote: > @@ -1828,7 +1827,6 @@ static int ibmvnic_set_mac(struct net_device *netdev, void *p) > int rc; > > rc = 0; > - ether_addr_copy(adapter->mac_addr, addr->sa_data); > if (adapter->state != VNIC_PROBED) > rc = __ibmvnic_set_mac(netdev, addr->sa_data); > If we just do this, in case state == VNIC_PROBED ibmvnic_set_mac() will do nothing - just return 0 without saving the user supplied MAC address.
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index 4dd3625a4fbc..c4c2b0e453fc 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -1817,7 +1817,6 @@ static int __ibmvnic_set_mac(struct net_device *netdev, u8 *dev_addr) mutex_unlock(&adapter->fw_lock); return 0; err: - ether_addr_copy(adapter->mac_addr, netdev->dev_addr); return rc; } @@ -1828,7 +1827,6 @@ static int ibmvnic_set_mac(struct net_device *netdev, void *p) int rc; rc = 0; - ether_addr_copy(adapter->mac_addr, addr->sa_data); if (adapter->state != VNIC_PROBED) rc = __ibmvnic_set_mac(netdev, addr->sa_data);
Jakub Kicinski brought up a concern in ibmvnic_set_mac(). ibmvnic_set_mac() does this: ether_addr_copy(adapter->mac_addr, addr->sa_data); if (adapter->state != VNIC_PROBED) rc = __ibmvnic_set_mac(netdev, addr->sa_data); So if state == VNIC_PROBED, the user can assign an invalid address to adapter->mac_addr, and ibmvnic_set_mac() will still return 0. The fix is to not update adapter->mac_addr in ibmvnic_set_mac and the error path of __ibmvnic_set_mac, and to update adpater->mac_addr in handle_change_mac_rsp after the change mac request is completed in VIOS. Fixes: 62740e97881c ("net/ibmvnic: Update MAC address settings after adapter reset") Cc: Jakub Kicinski <kuba@kernel.org> Signed-off-by: Lijun Pan <ljp@linux.ibm.com> --- drivers/net/ethernet/ibm/ibmvnic.c | 2 -- 1 file changed, 2 deletions(-)