From faf50e8fa39e4ff3a64418328bedcde6a89a77f2 Mon Sep 17 00:00:00 2001 From: Michael Brown Date: Thu, 8 Dec 2011 02:35:23 +0000 Subject: [PATCH] [pxe] Check for a valid PXE network device when applicable Very nasty things can happen if a NULL network device is used. Check that pxe_netdev is non-NULL at the applicable entry points, so that this type of problem gets reported to the caller rather than being allowed to crash the system. Signed-off-by: Michael Brown --- src/arch/i386/interface/pxe/pxe_preboot.c | 8 + src/arch/i386/interface/pxe/pxe_undi.c | 216 ++++++++++++++++++++-- 2 files changed, 213 insertions(+), 11 deletions(-) diff --git a/src/arch/i386/interface/pxe/pxe_preboot.c b/src/arch/i386/interface/pxe/pxe_preboot.c index 131ce8a8..4a9f7ed5 100644 --- a/src/arch/i386/interface/pxe/pxe_preboot.c +++ b/src/arch/i386/interface/pxe/pxe_preboot.c @@ -155,6 +155,14 @@ PXENV_EXIT_t pxenv_get_cached_info ( struct s_PXENV_GET_CACHED_INFO get_cached_info->Buffer.segment, get_cached_info->Buffer.offset, get_cached_info->BufferSize ); + /* Sanity check */ + if ( ! pxe_netdev ) { + DBGC ( &pxe_netdev, "PXENV_GET_CACHED_INFO called with no " + "network device\n" ); + get_cached_info->Status = PXENV_STATUS_UNDI_INVALID_STATE; + return PXENV_EXIT_FAILURE; + } + /* Sanity check */ idx = ( get_cached_info->PacketType - 1 ); if ( idx >= NUM_CACHED_INFOS ) { diff --git a/src/arch/i386/interface/pxe/pxe_undi.c b/src/arch/i386/interface/pxe/pxe_undi.c index 82fb683e..6b52aaac 100644 --- a/src/arch/i386/interface/pxe/pxe_undi.c +++ b/src/arch/i386/interface/pxe/pxe_undi.c @@ -58,11 +58,14 @@ struct net_device *pxe_netdev = NULL; * @v netdev Network device, or NULL */ void pxe_set_netdev ( struct net_device *netdev ) { + if ( pxe_netdev ) { netdev_rx_unfreeze ( pxe_netdev ); netdev_put ( pxe_netdev ); } + pxe_netdev = NULL; + if ( netdev ) pxe_netdev = netdev_get ( netdev ); } @@ -75,11 +78,14 @@ void pxe_set_netdev ( struct net_device *netdev ) { static int pxe_netdev_open ( void ) { int rc; + assert ( pxe_netdev != NULL ); + if ( ( rc = netdev_open ( pxe_netdev ) ) != 0 ) return rc; netdev_rx_freeze ( pxe_netdev ); netdev_irq ( pxe_netdev, 1 ); + return 0; } @@ -88,6 +94,8 @@ static int pxe_netdev_open ( void ) { * */ static void pxe_netdev_close ( void ) { + + assert ( pxe_netdev != NULL ); netdev_rx_unfreeze ( pxe_netdev ); netdev_irq ( pxe_netdev, 0 ); netdev_close ( pxe_netdev ); @@ -116,6 +124,14 @@ static void pxe_dump_mcast_list ( struct s_PXENV_UNDI_MCAST_ADDRESS *mcast ) { PXENV_EXIT_t pxenv_undi_startup ( struct s_PXENV_UNDI_STARTUP *undi_startup ) { DBGC ( &pxe_netdev, "PXENV_UNDI_STARTUP\n" ); + /* Sanity check */ + if ( ! pxe_netdev ) { + DBGC ( &pxe_netdev, "PXENV_UNDI_STARTUP called with no " + "network device\n" ); + undi_startup->Status = PXENV_STATUS_UNDI_INVALID_STATE; + return PXENV_EXIT_FAILURE; + } + undi_startup->Status = PXENV_STATUS_SUCCESS; return PXENV_EXIT_SUCCESS; } @@ -127,6 +143,15 @@ PXENV_EXIT_t pxenv_undi_startup ( struct s_PXENV_UNDI_STARTUP *undi_startup ) { PXENV_EXIT_t pxenv_undi_cleanup ( struct s_PXENV_UNDI_CLEANUP *undi_cleanup ) { DBGC ( &pxe_netdev, "PXENV_UNDI_CLEANUP\n" ); + /* Sanity check */ + if ( ! pxe_netdev ) { + DBGC ( &pxe_netdev, "PXENV_UNDI_CLEANUP called with no " + "network device\n" ); + undi_cleanup->Status = PXENV_STATUS_UNDI_INVALID_STATE; + return PXENV_EXIT_FAILURE; + } + + /* Close network device */ pxe_netdev_close(); undi_cleanup->Status = PXENV_STATUS_SUCCESS; @@ -142,6 +167,14 @@ PXENV_EXIT_t pxenv_undi_initialize ( struct s_PXENV_UNDI_INITIALIZE DBGC ( &pxe_netdev, "PXENV_UNDI_INITIALIZE protocolini %08x\n", undi_initialize->ProtocolIni ); + /* Sanity check */ + if ( ! pxe_netdev ) { + DBGC ( &pxe_netdev, "PXENV_UNDI_INITIALIZE called with no " + "network device\n" ); + undi_initialize->Status = PXENV_STATUS_UNDI_INVALID_STATE; + return PXENV_EXIT_FAILURE; + } + undi_initialize->Status = PXENV_STATUS_SUCCESS; return PXENV_EXIT_SUCCESS; } @@ -158,6 +191,15 @@ PXENV_EXIT_t pxenv_undi_reset_adapter ( struct s_PXENV_UNDI_RESET pxe_dump_mcast_list ( &undi_reset_adapter->R_Mcast_Buf ); DBGC ( &pxe_netdev, "\n" ); + /* Sanity check */ + if ( ! pxe_netdev ) { + DBGC ( &pxe_netdev, "PXENV_UNDI_RESET_ADAPTER called with no " + "network device\n" ); + undi_reset_adapter->Status = PXENV_STATUS_UNDI_INVALID_STATE; + return PXENV_EXIT_FAILURE; + } + + /* Close and reopen network device */ pxe_netdev_close(); if ( ( rc = pxe_netdev_open() ) != 0 ) { DBGC ( &pxe_netdev, "PXENV_UNDI_RESET_ADAPTER could not " @@ -178,6 +220,15 @@ PXENV_EXIT_t pxenv_undi_shutdown ( struct s_PXENV_UNDI_SHUTDOWN *undi_shutdown ) { DBGC ( &pxe_netdev, "PXENV_UNDI_SHUTDOWN\n" ); + /* Sanity check */ + if ( ! pxe_netdev ) { + DBGC ( &pxe_netdev, "PXENV_UNDI_SHUTDOWN called with no " + "network device\n" ); + undi_shutdown->Status = PXENV_STATUS_UNDI_INVALID_STATE; + return PXENV_EXIT_FAILURE; + } + + /* Close network device */ pxe_netdev_close(); undi_shutdown->Status = PXENV_STATUS_SUCCESS; @@ -196,6 +247,15 @@ PXENV_EXIT_t pxenv_undi_open ( struct s_PXENV_UNDI_OPEN *undi_open ) { pxe_dump_mcast_list ( &undi_open->R_Mcast_Buf ); DBGC ( &pxe_netdev, "\n" ); + /* Sanity check */ + if ( ! pxe_netdev ) { + DBGC ( &pxe_netdev, "PXENV_UNDI_OPEN called with no " + "network device\n" ); + undi_open->Status = PXENV_STATUS_UNDI_INVALID_STATE; + return PXENV_EXIT_FAILURE; + } + + /* Open network device */ if ( ( rc = pxe_netdev_open() ) != 0 ) { DBGC ( &pxe_netdev, "PXENV_UNDI_OPEN could not open %s: %s\n", pxe_netdev->name, strerror ( rc ) ); @@ -214,6 +274,15 @@ PXENV_EXIT_t pxenv_undi_open ( struct s_PXENV_UNDI_OPEN *undi_open ) { PXENV_EXIT_t pxenv_undi_close ( struct s_PXENV_UNDI_CLOSE *undi_close ) { DBGC ( &pxe_netdev, "PXENV_UNDI_CLOSE\n" ); + /* Sanity check */ + if ( ! pxe_netdev ) { + DBGC ( &pxe_netdev, "PXENV_UNDI_CLOSE called with no " + "network device\n" ); + undi_close->Status = PXENV_STATUS_UNDI_INVALID_STATE; + return PXENV_EXIT_FAILURE; + } + + /* Close network device */ pxe_netdev_close(); undi_close->Status = PXENV_STATUS_SUCCESS; @@ -230,7 +299,7 @@ PXENV_EXIT_t pxenv_undi_transmit ( struct s_PXENV_UNDI_TRANSMIT struct DataBlk *datablk; struct io_buffer *iobuf; struct net_protocol *net_protocol; - struct ll_protocol *ll_protocol = pxe_netdev->ll_protocol; + struct ll_protocol *ll_protocol; char destaddr[MAX_LL_ADDR_LEN]; const void *ll_dest; size_t len; @@ -239,6 +308,14 @@ PXENV_EXIT_t pxenv_undi_transmit ( struct s_PXENV_UNDI_TRANSMIT DBGC2 ( &pxe_netdev, "PXENV_UNDI_TRANSMIT" ); + /* Sanity check */ + if ( ! pxe_netdev ) { + DBGC ( &pxe_netdev, "PXENV_UNDI_TRANSMIT called with no " + "network device\n" ); + undi_transmit->Status = PXENV_STATUS_UNDI_INVALID_STATE; + return PXENV_EXIT_FAILURE; + } + /* Forcibly enable interrupts and freeze receive queue * processing at this point, to work around callers that never * call PXENV_UNDI_OPEN before attempting to use the UNDI API. @@ -299,6 +376,7 @@ PXENV_EXIT_t pxenv_undi_transmit ( struct s_PXENV_UNDI_TRANSMIT if ( net_protocol != NULL ) { /* Calculate destination address */ + ll_protocol = pxe_netdev->ll_protocol; if ( undi_transmit->XmitFlag == XMT_DESTADDR ) { copy_from_real ( destaddr, undi_transmit->DestAddr.segment, @@ -355,6 +433,15 @@ pxenv_undi_set_mcast_address ( struct s_PXENV_UNDI_SET_MCAST_ADDRESS pxe_dump_mcast_list ( &undi_set_mcast_address->R_Mcast_Buf ); DBGC ( &pxe_netdev, "\n" ); + /* Sanity check */ + if ( ! pxe_netdev ) { + DBGC ( &pxe_netdev, "PXENV_UNDI_SET_MCAST_ADDRESS called with " + "no network device\n" ); + undi_set_mcast_address->Status = + PXENV_STATUS_UNDI_INVALID_STATE; + return PXENV_EXIT_FAILURE; + } + undi_set_mcast_address->Status = PXENV_STATUS_SUCCESS; return PXENV_EXIT_SUCCESS; } @@ -366,8 +453,18 @@ pxenv_undi_set_mcast_address ( struct s_PXENV_UNDI_SET_MCAST_ADDRESS PXENV_EXIT_t pxenv_undi_set_station_address ( struct s_PXENV_UNDI_SET_STATION_ADDRESS *undi_set_station_address ) { - struct ll_protocol *ll_protocol = pxe_netdev->ll_protocol; + struct ll_protocol *ll_protocol; + /* Sanity check */ + if ( ! pxe_netdev ) { + DBGC ( &pxe_netdev, "PXENV_UNDI_SET_STATION_ADDRESS called " + "with no network device\n" ); + undi_set_station_address->Status = + PXENV_STATUS_UNDI_INVALID_STATE; + return PXENV_EXIT_FAILURE; + } + + ll_protocol = pxe_netdev->ll_protocol; DBGC ( &pxe_netdev, "PXENV_UNDI_SET_STATION_ADDRESS %s", ll_protocol->ntoa ( undi_set_station_address->StationAddress ) ); @@ -403,6 +500,15 @@ pxenv_undi_set_packet_filter ( struct s_PXENV_UNDI_SET_PACKET_FILTER DBGC ( &pxe_netdev, "PXENV_UNDI_SET_PACKET_FILTER %02x\n", undi_set_packet_filter->filter ); + /* Sanity check */ + if ( ! pxe_netdev ) { + DBGC ( &pxe_netdev, "PXENV_UNDI_SET_PACKET_FILTER called with " + "no network device\n" ); + undi_set_packet_filter->Status = + PXENV_STATUS_UNDI_INVALID_STATE; + return PXENV_EXIT_FAILURE; + } + /* Pretend that we succeeded, otherwise the 3Com DOS UNDI * driver refuses to load. (We ignore the filter value in the * PXENV_UNDI_OPEN call anyway.) @@ -418,20 +524,30 @@ pxenv_undi_set_packet_filter ( struct s_PXENV_UNDI_SET_PACKET_FILTER */ PXENV_EXIT_t pxenv_undi_get_information ( struct s_PXENV_UNDI_GET_INFORMATION *undi_get_information ) { - struct device *dev = pxe_netdev->dev; - struct ll_protocol *ll_protocol = pxe_netdev->ll_protocol; - size_t ll_addr_len = ll_protocol->ll_addr_len; + struct device *dev; + struct ll_protocol *ll_protocol; DBGC ( &pxe_netdev, "PXENV_UNDI_GET_INFORMATION" ); + /* Sanity check */ + if ( ! pxe_netdev ) { + DBGC ( &pxe_netdev, "PXENV_UNDI_GET_INFORMATION called with no " + "network device\n" ); + undi_get_information->Status = PXENV_STATUS_UNDI_INVALID_STATE; + return PXENV_EXIT_FAILURE; + } + + /* Fill in information */ + dev = pxe_netdev->dev; + ll_protocol = pxe_netdev->ll_protocol; undi_get_information->BaseIo = dev->desc.ioaddr; undi_get_information->IntNumber = ( netdev_irq_supported ( pxe_netdev ) ? dev->desc.irq : 0 ); /* Cheat: assume all cards can cope with this */ undi_get_information->MaxTranUnit = ETH_MAX_MTU; undi_get_information->HwType = ntohs ( ll_protocol->ll_proto ); - undi_get_information->HwAddrLen = ll_addr_len; - assert ( ll_addr_len <= + undi_get_information->HwAddrLen = ll_protocol->ll_addr_len; + assert ( ll_protocol->ll_addr_len <= sizeof ( undi_get_information->CurrentNodeAddress ) ); memcpy ( &undi_get_information->CurrentNodeAddress, pxe_netdev->ll_addr, @@ -462,16 +578,25 @@ PXENV_EXIT_t pxenv_undi_get_statistics ( struct s_PXENV_UNDI_GET_STATISTICS *undi_get_statistics ) { DBGC ( &pxe_netdev, "PXENV_UNDI_GET_STATISTICS" ); + /* Sanity check */ + if ( ! pxe_netdev ) { + DBGC ( &pxe_netdev, "PXENV_UNDI_GET_STATISTICS called with no " + "network device\n" ); + undi_get_statistics->Status = PXENV_STATUS_UNDI_INVALID_STATE; + return PXENV_EXIT_FAILURE; + } + + /* Report statistics */ undi_get_statistics->XmtGoodFrames = pxe_netdev->tx_stats.good; undi_get_statistics->RcvGoodFrames = pxe_netdev->rx_stats.good; undi_get_statistics->RcvCRCErrors = pxe_netdev->rx_stats.bad; undi_get_statistics->RcvResourceErrors = pxe_netdev->rx_stats.bad; - DBGC ( &pxe_netdev, " txok %d rxok %d rxcrc %d rxrsrc %d\n", undi_get_statistics->XmtGoodFrames, undi_get_statistics->RcvGoodFrames, undi_get_statistics->RcvCRCErrors, undi_get_statistics->RcvResourceErrors ); + undi_get_statistics->Status = PXENV_STATUS_SUCCESS; return PXENV_EXIT_SUCCESS; } @@ -484,6 +609,15 @@ PXENV_EXIT_t pxenv_undi_clear_statistics ( struct s_PXENV_UNDI_CLEAR_STATISTICS *undi_clear_statistics ) { DBGC ( &pxe_netdev, "PXENV_UNDI_CLEAR_STATISTICS\n" ); + /* Sanity check */ + if ( ! pxe_netdev ) { + DBGC ( &pxe_netdev, "PXENV_UNDI_CLEAR_STATISTICS called with " + "no network device\n" ); + undi_clear_statistics->Status = PXENV_STATUS_UNDI_INVALID_STATE; + return PXENV_EXIT_FAILURE; + } + + /* Clear statistics */ memset ( &pxe_netdev->tx_stats, 0, sizeof ( pxe_netdev->tx_stats ) ); memset ( &pxe_netdev->rx_stats, 0, sizeof ( pxe_netdev->rx_stats ) ); @@ -500,6 +634,14 @@ PXENV_EXIT_t pxenv_undi_initiate_diags ( struct s_PXENV_UNDI_INITIATE_DIAGS *undi_initiate_diags ) { DBGC ( &pxe_netdev, "PXENV_UNDI_INITIATE_DIAGS failed: unsupported\n" ); + /* Sanity check */ + if ( ! pxe_netdev ) { + DBGC ( &pxe_netdev, "PXENV_UNDI_INITIATE_DIAGS called with no " + "network device\n" ); + undi_initiate_diags->Status = PXENV_STATUS_UNDI_INVALID_STATE; + return PXENV_EXIT_FAILURE; + } + undi_initiate_diags->Status = PXENV_STATUS_UNSUPPORTED; return PXENV_EXIT_FAILURE; } @@ -514,6 +656,14 @@ PXENV_EXIT_t pxenv_undi_force_interrupt ( struct s_PXENV_UNDI_FORCE_INTERRUPT DBGC ( &pxe_netdev, "PXENV_UNDI_FORCE_INTERRUPT failed: unsupported\n" ); + /* Sanity check */ + if ( ! pxe_netdev ) { + DBGC ( &pxe_netdev, "PXENV_UNDI_FORCE_INTERRUPT called with no " + "network device\n" ); + undi_force_interrupt->Status = PXENV_STATUS_UNDI_INVALID_STATE; + return PXENV_EXIT_FAILURE; + } + undi_force_interrupt->Status = PXENV_STATUS_UNSUPPORTED; return PXENV_EXIT_FAILURE; } @@ -525,13 +675,24 @@ PXENV_EXIT_t pxenv_undi_force_interrupt ( struct s_PXENV_UNDI_FORCE_INTERRUPT PXENV_EXIT_t pxenv_undi_get_mcast_address ( struct s_PXENV_UNDI_GET_MCAST_ADDRESS *undi_get_mcast_address ) { - struct ll_protocol *ll_protocol = pxe_netdev->ll_protocol; + struct ll_protocol *ll_protocol; struct in_addr ip = { .s_addr = undi_get_mcast_address->InetAddr }; int rc; DBGC ( &pxe_netdev, "PXENV_UNDI_GET_MCAST_ADDRESS %s", inet_ntoa ( ip ) ); + /* Sanity check */ + if ( ! pxe_netdev ) { + DBGC ( &pxe_netdev, "PXENV_UNDI_GET_MCAST_ADDRESS called with " + "no network device\n" ); + undi_get_mcast_address->Status = + PXENV_STATUS_UNDI_INVALID_STATE; + return PXENV_EXIT_FAILURE; + } + + /* Hash address using the network device's link-layer protocol */ + ll_protocol = pxe_netdev->ll_protocol; if ( ( rc = ll_protocol->mc_hash ( AF_INET, &ip, undi_get_mcast_address->MediaAddr ))!=0){ DBGC ( &pxe_netdev, " failed: %s\n", strerror ( rc ) ); @@ -551,13 +712,22 @@ pxenv_undi_get_mcast_address ( struct s_PXENV_UNDI_GET_MCAST_ADDRESS */ PXENV_EXIT_t pxenv_undi_get_nic_type ( struct s_PXENV_UNDI_GET_NIC_TYPE *undi_get_nic_type ) { - struct device *dev = pxe_netdev->dev; + struct device *dev; DBGC ( &pxe_netdev, "PXENV_UNDI_GET_NIC_TYPE" ); + /* Sanity check */ + if ( ! pxe_netdev ) { + DBGC ( &pxe_netdev, "PXENV_UNDI_GET_NIC_TYPE called with " + "no network device\n" ); + undi_get_nic_type->Status = PXENV_STATUS_UNDI_INVALID_STATE; + return PXENV_EXIT_FAILURE; + } + + /* Fill in information */ memset ( &undi_get_nic_type->info, 0, sizeof ( undi_get_nic_type->info ) ); - + dev = pxe_netdev->dev; switch ( dev->desc.bus_type ) { case BUS_TYPE_PCI: { struct pci_nic_info *info = &undi_get_nic_type->info.pci; @@ -615,6 +785,14 @@ PXENV_EXIT_t pxenv_undi_get_iface_info ( struct s_PXENV_UNDI_GET_IFACE_INFO *undi_get_iface_info ) { DBGC ( &pxe_netdev, "PXENV_UNDI_GET_IFACE_INFO" ); + /* Sanity check */ + if ( ! pxe_netdev ) { + DBGC ( &pxe_netdev, "PXENV_UNDI_GET_IFACE_INFO called with " + "no network device\n" ); + undi_get_iface_info->Status = PXENV_STATUS_UNDI_INVALID_STATE; + return PXENV_EXIT_FAILURE; + } + /* Just hand back some info, doesn't really matter what it is. * Most PXE stacks seem to take this approach. */ @@ -645,6 +823,14 @@ PXENV_EXIT_t pxenv_undi_get_state ( struct s_PXENV_UNDI_GET_STATE *undi_get_state ) { DBGC ( &pxe_netdev, "PXENV_UNDI_GET_STATE failed: unsupported\n" ); + /* Sanity check */ + if ( ! pxe_netdev ) { + DBGC ( &pxe_netdev, "PXENV_UNDI_GET_STATE called with " + "no network device\n" ); + undi_get_state->Status = PXENV_STATUS_UNDI_INVALID_STATE; + return PXENV_EXIT_FAILURE; + } + undi_get_state->Status = PXENV_STATUS_UNSUPPORTED; return PXENV_EXIT_FAILURE; }; @@ -671,6 +857,14 @@ PXENV_EXIT_t pxenv_undi_isr ( struct s_PXENV_UNDI_ISR *undi_isr ) { */ DBGC2 ( &pxenv_undi_isr, "PXENV_UNDI_ISR" ); + /* Sanity check */ + if ( ! pxe_netdev ) { + DBGC ( &pxe_netdev, "PXENV_UNDI_ISR called with " + "no network device\n" ); + undi_isr->Status = PXENV_STATUS_UNDI_INVALID_STATE; + return PXENV_EXIT_FAILURE; + } + /* Just in case some idiot actually looks at these fields when * we weren't meant to fill them in... */