[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 <mcb30@ipxe.org>

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 ) {

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 device *dev;
-	struct ll_protocol *ll_protocol = pxe_netdev->ll_protocol;
+	struct ll_protocol *ll_protocol;
-	size_t ll_addr_len = ll_protocol->ll_addr_len;
 
 	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;
+	undi_get_information->HwAddrLen = ll_protocol->ll_addr_len;
-	assert ( 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...
 	 */