Re: kern/44144 (ifconfig causes assertion failure in vfs_lookup.c)

[David Holland <dholland-bugs%netbsd.org@localhost>]

The following reply was made to PR kern/44144; it has been noted by GNATS.

From: David Holland <dholland-bugs%netbsd.org@localhost>
To: gnats-bugs%netbsd.org@localhost
Cc: 
Subject: Re: kern/44144 (ifconfig causes assertion failure in vfs_lookup.c)
Date: Mon, 30 May 2011 03:44:25 +0000

 On Sun, Dec 26, 2010 at 10:20:04PM +0000, David Holland wrote:
  >   >  Thank you; I'll try it tomorrow.
  >  
  >  Let me know when/if I should commit...
 
 Slightly updated patch (the previous version now rejects):
 
 Index: if_wpi.c
 ===================================================================
 RCS file: /cvsroot/src/sys/dev/pci/if_wpi.c,v
 retrieving revision 1.49
 diff -u -p -r1.49 if_wpi.c
 --- if_wpi.c   2 Apr 2011 08:11:31 -0000       1.49
 +++ if_wpi.c   30 May 2011 03:42:25 -0000
 @@ -418,8 +418,12 @@ wpi_detach(device_t self, int flags __un
  
        if (sc->fw_used) {
                mutex_enter(&wpi_firmware_mutex);
 -              if (--wpi_firmware_users == 0)
 +              if (--wpi_firmware_users == 0) {
 +                      KASSERT(wpi_firmware_image != NULL);
                        firmware_free(wpi_firmware_image, wpi_firmware_size);
 +                      wpi_firmware_image = NULL;
 +                      wpi_firmware_size = 0;
 +              }
                mutex_exit(&wpi_firmware_mutex);
        }
  
 @@ -1134,6 +1138,7 @@ wpi_load_microcode(struct wpi_softc *sc,
  static int
  wpi_cache_firmware(struct wpi_softc *sc)
  {
 +      static const char fwname[] = "iwlwifi-3945.ucode";
        firmware_handle_t fw;
        int error;
  
 @@ -1147,9 +1152,10 @@ wpi_cache_firmware(struct wpi_softc *sc)
        }
  
        /* load firmware image from disk */
 -      if ((error = firmware_open("if_wpi","iwlwifi-3945.ucode", &fw)) != 0) {
 -              aprint_error_dev(sc->sc_dev, "could not read firmware file\n");
 -              goto fail1;
 +      if ((error = firmware_open("if_wpi", fwname, &fw)) != 0) {
 +              aprint_error_dev(sc->sc_dev,
 +                  "Could not open firmware file %s\n", fwname);
 +              goto fail0;
        }
  
        wpi_firmware_size = firmware_get_size(fw);
 @@ -1158,29 +1164,33 @@ wpi_cache_firmware(struct wpi_softc *sc)
            WPI_FW_MAIN_TEXT_MAXSZ + WPI_FW_MAIN_DATA_MAXSZ +
            WPI_FW_INIT_TEXT_MAXSZ + WPI_FW_INIT_DATA_MAXSZ +
            WPI_FW_BOOT_TEXT_MAXSZ) {
 -              aprint_error_dev(sc->sc_dev, "invalid firmware file\n");
 +              aprint_error_dev(sc->sc_dev, "Invalid firmware file %s\n",
 +                  fwname);
                error = EFBIG;
                goto fail1;
        }
  
        if (wpi_firmware_size < sizeof (struct wpi_firmware_hdr)) {
                aprint_error_dev(sc->sc_dev,
 -                  "truncated firmware header: %zu bytes\n",
 -                  wpi_firmware_size);
 +                  "Truncated firmware header in %s: %zu bytes\n",
 +                  fwname, wpi_firmware_size);
                error = EINVAL;
 -              goto fail2;
 +              goto fail1;
        }
  
        wpi_firmware_image = firmware_malloc(wpi_firmware_size);
        if (wpi_firmware_image == NULL) {
 -              aprint_error_dev(sc->sc_dev, "not enough memory to stock 
firmware\n");
 +              aprint_error_dev(sc->sc_dev,
 +                  "Not enough memory for firmware %s\n", fwname);
                error = ENOMEM;
                goto fail1;
        }
  
 -      if ((error = firmware_read(fw, 0, wpi_firmware_image, 
wpi_firmware_size)) != 0) {
 -              aprint_error_dev(sc->sc_dev, "can't get firmware\n");
 -              goto fail2;
 +      error = firmware_read(fw, 0, wpi_firmware_image, wpi_firmware_size);
 +      if (error != 0) {
 +              aprint_error_dev(sc->sc_dev,
 +                  "Error reading firmware file %s\n", fwname);
 +              goto fail1;
        }
  
        sc->fw_used = true;
 @@ -1189,12 +1199,16 @@ wpi_cache_firmware(struct wpi_softc *sc)
  
        return 0;
  
 -fail2:
 -      firmware_free(wpi_firmware_image, wpi_firmware_size);
  fail1:
        firmware_close(fw);
 -      if (--wpi_firmware_users == 0)
 +fail0:
 +      --wpi_firmware_users;
 +      KASSERT(wpi_firmware_users == 0);
 +      if (wpi_firmware_image != NULL) {
                firmware_free(wpi_firmware_image, wpi_firmware_size);
 +              wpi_firmware_image = NULL;
 +              wpi_firmware_size = 0;
 +      }
        mutex_exit(&wpi_firmware_mutex);
        return error;
  }
 @@ -3092,7 +3106,8 @@ wpi_init(struct ifnet *ifp)
        WPI_WRITE(sc, WPI_UCODE_CLR, WPI_RADIO_OFF);
  
        if ((error = wpi_load_firmware(sc)) != 0) {
 -              aprint_error_dev(sc->sc_dev, "could not load firmware\n");
 +              /* wpi_load_firmware already prints a message on error */
 +              /*aprint_error_dev(sc->sc_dev, "could not load firmware\n");*/
                goto fail1;
        }
  
 
 
 -- 
 David A. Holland
 dholland%netbsd.org@localhost