Message ID | 20201027220456.71450-1-ljp@linux.ibm.com |
---|---|
State | Accepted |
Delegated to: | David Miller |
Headers | show |
Series | [net,v3] ibmvnic: fix ibmvnic_set_mac | 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 | fail | Errors and warnings before: 4 this patch: 4 |
jkicinski/kdoc | success | Errors and warnings before: 18 this patch: 18 |
jkicinski/verify_fixes | success | Link |
jkicinski/checkpatch | fail | Link |
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, 27 Oct 2020 17:04:56 -0500 Lijun Pan wrote: > 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 validate ethernet address at the beginning of > ibmvnic_set_mac(), and move the ether_addr_copy to > the case of "adapter->state != VNIC_PROBED". > > 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> Applied, thanks.
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index 4dd3625a4fbc..660761538c55 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -1828,9 +1828,13 @@ 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) + if (!is_valid_ether_addr(addr->sa_data)) + return -EADDRNOTAVAIL; + + if (adapter->state != VNIC_PROBED) { + ether_addr_copy(adapter->mac_addr, addr->sa_data); rc = __ibmvnic_set_mac(netdev, addr->sa_data); + } return rc; }
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 validate ethernet address at the beginning of ibmvnic_set_mac(), and move the ether_addr_copy to the case of "adapter->state != VNIC_PROBED". 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> --- v2: change the subject from v1's "ibmvnic: no need to update adapter->mac_addr before it completes" handle adapter->state==VNIC_PROBED case in else statement. v3: take Maciej Fijalkowski's advice. drivers/net/ethernet/ibm/ibmvnic.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)