Bug 197139
| Summary: | Double cleanup in igb_attach | ||
|---|---|---|---|
| Product: | Base System | Reporter: | Sreekanth Rupavatharam <rupavath> |
| Component: | kern | Assignee: | Eric Joyner <erj> |
| Status: | Closed FIXED | ||
| Severity: | Affects Some People | CC: | erj, sbruno |
| Priority: | --- | Keywords: | IntelNetworking, patch |
| Version: | CURRENT | ||
| Hardware: | i386 | ||
| OS: | Any | ||
This is still applicable. A commit references this bug: Author: sbruno Date: Fri May 6 17:00:45 UTC 2016 New revision: 299188 URL: https://svnweb.freebsd.org/changeset/base/299188 Log: Since igb_detach() cleans up all the data structures that will be free'd by the functions following its call, we can simply return instead of crashing and burning in the event of igb_detach() failing. PR: 197139 Submitted by: rupavath@juniper.net MFC after: 2 weeks Changes: head/sys/dev/e1000/if_igb.c A commit references this bug: Author: sbruno Date: Fri Jul 22 03:19:50 UTC 2016 New revision: 303175 URL: https://svnweb.freebsd.org/changeset/base/303175 Log: MFC r299188 Since igb_detach() cleans up all the data structures that will be free'd by the functions following its call, we can simply return instead of crashing and burning in the event of igb_detach() failing. PR: 197139 Changes: stable/10/sys/dev/e1000/if_igb.c Committed back in 2016. |
igb_attach has this code err_late: igb_detach(dev); igb_free_transmit_structures(adapter); igb_free_receive_structures(adapter); igb_release_hw_control(adapter); err_pci: igb_free_pci_resources(adapter); if (adapter->ifp != NULL) if_free(adapter->ifp); free(adapter->mta, M_DEVBUF); IGB_CORE_LOCK_DESTROY(adapter); return (error); However, I see that igb_detach does all this cleanup when it's successfully completed. Only exception I see is that it return EBUSY if vlantrunk is in use /* Make sure VLANS are not using driver */ if (if_vlantrunkinuse(ifp)) { device_printf(dev,"Vlan in use, detach first\n"); return (EBUSY); } This results in duplicate cleanup in igb_attach and can cause crashes. The fix would be the following. Index: if_igb.c =================================================================== --- if_igb.c (revision 298053) +++ if_igb.c (working copy) @@ -723,7 +723,8 @@ igb_attach(device_t dev) return (0); err_late: - igb_detach(dev); + if(igb_detach(dev) == 0) /* igb_detach did the cleanup */ + return(error); igb_free_transmit_structures(adapter); igb_free_receive_structures(adapter); igb_release_hw_control(adapter); The issue exists in stable/10 as well as current.