PVS-Studio in Linux-Interna durchwühlt (3.18.1)

    Linux und PVS-Studio
    Mitautor: Svyatoslav Razmyslov SvyatoslavMC .

    Zu Werbezwecken haben wir uns entschlossen, den Linux-Kernel mit unserem statischen Code-Analyzer zu überprüfen. Diese Aufgabe ist wegen ihrer Komplexität interessant. Linux-Quellcodes wurden nicht getestet und verifiziert. Zumindest etwas Neues zu finden, ist daher eine sehr schwierige Aufgabe. Aber wenn es klappt, wird es eine gute Werbebotschaft über die Fähigkeiten des PVS-Studio-Analysators sein.

    Was haben wir überprüft?


    Der Linux-Kernel stammt aus dem Linux-Kernel-Archiv . Aktueller stabiler Kernel überprüft 3.18.1 .

    Als ich diesen Artikel schreibe, ist der Kernel 3.19-rc1 bereits erschienen. Das Überprüfen eines Projekts und das Schreiben eines Artikels nimmt leider viel Zeit in Anspruch. Wir werden uns daher damit begnügen, nicht die neueste Version zu prüfen.

    Für diejenigen, die sagen, dass wir die neueste Version hätten prüfen sollen, möchte ich Folgendes beantworten.
    1. Wir prüfen regelmäßig eine Vielzahl von Projekten . Neben der kostenlosen Prüfung von Projekten haben wir noch viele andere Aufgaben. Es gibt also keine Möglichkeit, noch einmal von vorne zu beginnen, nur weil eine neue Version erschienen ist. Wir können also nie etwas veröffentlichen :).
    2. 99% der gefundenen Fehler blieben an ihrer Stelle. Dieser Artikel kann also sicher verwendet werden, um den Linux-Kernel-Code leicht zu verbessern.
    3. Gegenstand des Artikels ist die PVS-Studio-Werbung. Wenn wir Fehler in der Entwurfsversion X finden, können wir auch in Version Y etwas finden. Unsere Prüfungen sind recht oberflächlich (wir kennen das Projekt nicht) und ihr Ziel ist es, solche Artikel zu schreiben. Der eigentliche Vorteil des Projekts ist der Erwerb einer Lizenz für PVS-Studio und dessen regelmäßige Nutzung.

    Da haben wir nachgesehen


    Um den Kernel zu überprüfen, haben wir den statischen Code Analyzer von PVS-Studio Version 5.21 verwendet .

    Das Ubuntu-14.04 Distribution Kit wurde zum Testen des Linux-Kernels verwendet, für den es viele detaillierte Anweisungen zum Konfigurieren und Erstellen des Kernels gab. Der Analyzer überprüft die vorverarbeiteten Dateien , die am besten für erfolgreich kompilierte Dateien eingehen. Das Erstellen eines Projekts ist daher eine der wichtigsten Phasen der Überprüfung.

    Als Nächstes wurde ein kleines Dienstprogramm in C ++ geschrieben, das für jeden ausgeführten Compilerprozess die Befehlszeile, das aktuelle Verzeichnis und die Umgebungsvariablen speichert. Leser, die mit PVS-Studio-Produkten vertraut sind, sollten sich sofort an das Dienstprogramm PVS-Studio Standalone erinnern., mit dem Sie jedes Projekt für Windows überprüfen können. Dort wird WinAPI für den Zugriff auf die Prozesse verwendet. Daher wurde dieser Überwachungsmechanismus für Linux neu geschrieben, der Rest des Codes für den Start der Vorverarbeitung und Überprüfung wurde vollständig portiert und die Überprüfung des Linux-Kernels war nur eine Frage der Zeit.

    Sicherheit am Anfang


    Irgendwie ist es vorgekommen, dass der PVS-Studio Analyzer nur als Werkzeug zur Fehlersuche wahrgenommen wird. Aber niemand achtet darauf, dass PVS-Studio eine Reihe von Schwachstellen erkennen kann. Natürlich sind wir selbst dafür verantwortlich. Es ist notwendig, die Situation ein wenig zu korrigieren.

    Sie können die Meldungen, die PVS-Studio ausgibt, unterschiedlich wahrnehmen. Zum Beispiel ist es auf der einen Seite ein Tippfehler und auf der anderen Seite kann es eine Sicherheitslücke sein. Alles hängt vom Standpunkt ab.

    Ich möchte vorschlagen, einige Warnungen zu berücksichtigen, die PVS-Studio bei der Analyse von Linux ausgegeben hat. Ich sage nicht, dass der Analyzer eine Sicherheitslücke in Linux gefunden hat. Aber die fraglichen Warnungen hätten dies gut tun können.

    Gefährliche Verwendung der Funktion memcmp ()


    static unsigned char eprom_try_esi(
      struct atm_dev *dev, unsigned short cmd,
      int offset, int swap)
    {
      unsigned char buf[ZEPROM_SIZE];
      struct zatm_dev *zatm_dev;
      int i;
      zatm_dev = ZATM_DEV(dev);
      for (i = 0; i < ZEPROM_SIZE; i += 2) {
        eprom_set(zatm_dev,ZEPROM_CS,cmd); /* select EPROM */
        eprom_put_bits(zatm_dev,ZEPROM_CMD_READ,ZEPROM_CMD_LEN,cmd);
        eprom_put_bits(zatm_dev,i >> 1,ZEPROM_ADDR_LEN,cmd);
        eprom_get_byte(zatm_dev,buf+i+swap,cmd);
        eprom_get_byte(zatm_dev,buf+i+1-swap,cmd);
        eprom_set(zatm_dev,0,cmd); /* deselect EPROM */
      }
      memcpy(dev->esi,buf+offset,ESI_LEN);
      return memcmp(dev->esi,"\0\0\0\0\0",ESI_LEN);
    }

    PVS-Studio Warnung: V642 Das Speichern des Ergebnisses der 'memcmp'-Funktion in der Variablen vom Typ' unsigned char 'ist unangemessen. Die signifikanten Bits könnten verloren gehen und die Logik des Programms verletzen. zatm.c 1168

    Beachten Sie die ' return' -Anweisung ganz am Ende des Funktionskörpers.

    Die Funktion 'memcmp' gibt die folgenden Werte vom Typ 'int' zurück:
    • <0 - buf1 kleiner als buf2;
    • 0 - buf1 identisch mit buf2;
    • > 0 - buf1 größer als buf2;

    Beachten Sie:
    • "> 0" bedeutet eine beliebige Zahl, überhaupt keine 1;
    • "<0", das ist nicht unbedingt -1.
    Die zurückgegebenen Werte können -100, 2, 3, 100, 256, 1024, 5555 usw. sein. Dies bedeutet, dass dieses Ergebnis nicht in den Typ 'unsigned char' konvertiert werden kann (dies ist der Typ, den die Funktion zurückgibt).

    Infolge einer impliziten Typkonvertierung können signifikante Bits verworfen werden, was zu einer Verletzung der Programmausführungslogik führt.

    Die Gefahr eines solchen Fehlers besteht darin, dass der Rückgabewert von der Architektur und der Implementierung einer bestimmten Funktion auf dieser Architektur abhängen kann. Beispielsweise wird das Programm in der 32-Bit-Version korrekt und in der 64-Bit-Version falsch funktionieren.

    Na und? Stellen Sie sich vor, etwas im Zusammenhang mit EPROM wird falsch überprüft. Ein Fehler, aber was hat die Sicherheitslücke damit zu tun?

    Und die Tatsache, dass die V642-Diagnose eine Schwachstelle aufdecken kann! Glaube nicht Bitte, hier ist der identische Code von MySQL / MariaDB.
    typedef char my_bool;
    ...
    my_bool check(...) {
      return memcmp(...);
    }

    Nicht PVS-Studio hat diesen Fehler gefunden. Aber er konnte.

    Dieser Fehler verursachte eine schwerwiegende Sicherheitslücke in MySQL / MariaDB vor den Versionen 5.1.61, 5.2.11, 5.3.5, 5.5.22. Die Quintessenz ist, dass beim Verbinden eines MySQL / MariaDB-Benutzers ein Token (SHA aus Kennwort und Hash) berechnet wird, das mit dem erwarteten Wert der Funktion 'memcmp' verglichen wird. Auf einigen Plattformen liegt der Rückgabewert möglicherweise außerhalb des Bereichs [-128..127]. Infolgedessen gibt die Hash-Vergleichsprozedur mit dem erwarteten Wert in einem von 256 Fällen immer den Wert 'true' zurück, unabhängig vom Hash. Infolgedessen kann der Angreifer mit einem einfachen Bash-Befehl als Root auf den anfälligen MySQL-Server zugreifen, auch wenn er das Kennwort nicht kennt. Der Grund dafür war der oben gezeigte Code in der Datei 'sql / password.c'. Eine ausführlichere Beschreibung dieses Problems finden Sie hier:Sicherheitslücke in MySQL / MariaDB .

    Kehren wir jetzt zu Linux zurück. Hier ist ein weiterer gefährlicher Code:
    void sci_controller_power_control_queue_insert(....)
    {
      ....
      for (i = 0; i < SCI_MAX_PHYS; i++) {
        u8 other;
        current_phy = &ihost->phys[i];
        other = memcmp(current_phy->frame_rcvd.iaf.sas_addr,
                       iphy->frame_rcvd.iaf.sas_addr,
                       sizeof(current_phy->frame_rcvd.iaf.sas_addr));
        if (current_phy->sm.current_state_id == SCI_PHY_READY &&
            current_phy->protocol == SAS_PROTOCOL_SSP &&
            other == 0) {
          sci_phy_consume_power_handler(iphy);
          break;
        }
      }
      ....
    }

    PVS-Studio Warnung: V642 Das Speichern des Ergebnisses der 'memcmp'-Funktion in der Variablen vom Typ' unsigned char 'ist unangemessen. Die signifikanten Bits könnten verloren gehen und die Logik des Programms verletzen. host.c 1846

    Das Ergebnis der Funktion memcmp () wird in der anderen Variablen abgelegt, die vom Typ char ohne Vorzeichen ist. Ich glaube nicht, dass dies irgendeine Art von Sicherheitslücke ist, aber die Arbeit des SCSI-Controllers ist in Gefahr.

    Ein paar weitere dieser Orte finden Sie hier:
    • V642 Das Speichern des Ergebnisses der 'memcmp'-Funktion in der Variablen vom Typ' unsigned char 'ist unangemessen. Die signifikanten Bits könnten verloren gehen und die Logik des Programms verletzen. zatm.c 1168
    • V642 Das Speichern des Ergebnisses der 'memcmp'-Funktion in der Variablen vom Typ' unsigned char 'ist unangemessen. Die signifikanten Bits könnten verloren gehen und die Logik des Programms verletzen. host.c 1789

    Gefährliche Verwendung der Funktion memset ()


    Wir suchen weiterhin nach gefährlichen Orten. Wenden wir uns Funktionen zu, die private Daten „bereinigen“. Dies sind normalerweise verschiedene Verschlüsselungsfunktionen. Leider wird das Zurücksetzen des Speichers nicht immer korrekt geschrieben. Und dann können Sie ein äußerst unangenehmes Ergebnis erzielen. Mehr über diese unangenehmen Momente erfahren Sie in diesem Artikel: " Gedächtnis überschreiben - warum? ".

    Betrachten Sie das Beispiel eines falschen Codes:
    static int crypt_iv_tcw_whitening(....)
    {
      ....
      u8 buf[TCW_WHITENING_SIZE];
      ....
      out:
      memset(buf, 0, sizeof(buf));
      return r;
    }

    PVS-Studio Warnung: V597 Der Compiler könnte den 'memset'-Funktionsaufruf löschen, der zum Leeren des' buf'-Puffers verwendet wird. Die Funktion RtlSecureZeroMemory () sollte zum Löschen der privaten Daten verwendet werden. dm-crypt.c 708

    Auf den ersten Blick ist alles in Ordnung. Die Funktion crypt_iv_tcw_whitening () hat dem Stapel einen temporären Puffer zugewiesen, etwas verschlüsselt und den Puffer dann mit privaten Daten zurückgesetzt, indem sie die Funktion memset () aufruft. Tatsächlich wird der Funktionsaufruf memset () jedoch während der Optimierung vom Compiler gelöscht. Aus Sicht von C / C ++ wird der Puffer nach dem Nullsetzen in keiner Weise verwendet. Der Puffer kann also nicht annulliert werden.

    Gleichzeitig ist es äußerst schwierig, einen solchen Fehler zu bemerken. Einen Komponententest zu schreiben ist schwierig. Unter dem Debugger ist ein solcher Fehler ebenfalls nicht sichtbar (in der Debug-Version wird die Memset-Funktion aufgerufen).

    Gleichzeitig möchte ich betonen. Dies ist kein "theoretisch mögliches Compilerverhalten", sondern sehr praktisch. Compiler entfernen wirklich den Funktionsaufruf memset (). Mehr dazu erfahren Sie in der Beschreibung der V597- Diagnose .

    PVS-Studio empfiehlt zu Unrecht die Verwendung der Funktion RtlSecureZeroMemory (), da diese auf Windows ausgerichtet ist. Unter Linux gibt es natürlich keine solche Funktion. Die Hauptsache ist jedoch, zu warnen, und die Auswahl der richtigen Funktion ist nicht länger schwierig.

    Das Folgende ist ein ähnliches Beispiel:
    static int sha384_ssse3_final(struct shash_desc *desc, u8 *hash)
    {
      u8 D[SHA512_DIGEST_SIZE];
      sha512_ssse3_final(desc, D);
      memcpy(hash, D, SHA384_DIGEST_SIZE);
      memset(D, 0, SHA512_DIGEST_SIZE);
      return 0;
    }

    PVS-Studio Warnung: V597 Der Compiler könnte den 'memset'-Funktionsaufruf löschen, der zum Leeren des' D'-Puffers verwendet wird. Die Funktion RtlSecureZeroMemory () sollte zum Löschen der privaten Daten verwendet werden. sha512_ssse3_glue.c 222 Nachfolgend sehen Sie

    ein Beispiel, in dem 4 Puffer möglicherweise nicht sofort zurückgesetzt werden: keydvt_out, keydvt_in, ccm_n, mic. Der Code stammt aus der Datei security.c (Zeilen 525 - 528).
    int wusb_dev_4way_handshake(....)
    {
      ....
      struct aes_ccm_nonce ccm_n;
      u8 mic[8];
      struct wusb_keydvt_in keydvt_in;
      struct wusb_keydvt_out keydvt_out;
      ....
    error_dev_update_address:
    error_wusbhc_set_gtk:
    error_wusbhc_set_ptk:
    error_hs3:
    error_hs2:
    error_hs1:
      memset(hs, 0, 3*sizeof(hs[0]));
      memset(&keydvt_out, 0, sizeof(keydvt_out));
      memset(&keydvt_in, 0, sizeof(keydvt_in));
      memset(&ccm_n, 0, sizeof(ccm_n));
      memset(mic, 0, sizeof(mic));
      if (result < 0)
        wusb_dev_set_encryption(usb_dev, 0);
    error_dev_set_encryption:
      kfree(hs);
    error_kzalloc:
      return result;
      ....
    }

    Und das letzte Beispiel, in dem eine Art Passwort gespeichert bleibt:
    int
    E_md4hash(const unsigned char *passwd, unsigned char *p16,
      const struct nls_table *codepage)
    {
      int rc;
      int len;
      __le16 wpwd[129];
      /* Password cannot be longer than 128 characters */
      if (passwd) /* Password must be converted to NT unicode */
        len = cifs_strtoUTF16(wpwd, passwd, 128, codepage);
      else {
        len = 0;
        *wpwd = 0; /* Ensure string is null terminated */
      }
      rc = mdfour(p16, (unsigned char *) wpwd, len * sizeof(__le16));
      memset(wpwd, 0, 129 * sizeof(__le16));
      return rc;
    }

    PVS-Studio Warnung: V597 Der Compiler könnte den 'memset'-Funktionsaufruf löschen, der zum Leeren des' wpwd'-Puffers verwendet wird. Die Funktion RtlSecureZeroMemory () sollte zum Löschen der privaten Daten verwendet werden. smbencrypt.c 224

    Ich werde darauf näher eingehen . 3 weitere erfolglose memset () finden Sie hier:
    • sha256_ssse3_glue.c 214
    • dev-sysfs.c 104
    • qp.c 143

    Gefährliche Schecks


    Der PVS-Studio-Analysator verfügt über eine V595-Diagnose, die Situationen erkennt, in denen der Zeiger zu Beginn dereferenziert und dann auf Gleichheit NULL überprüft wird. Manchmal ist bei dieser Diagnose alles einfach und klar. Betrachten Sie diesen einfachen Fall:
    static int tc_ctl_action(struct sk_buff *skb, struct nlmsghdr *n)
    {
      struct net *net = sock_net(skb->sk);
      struct nlattr *tca[TCA_ACT_MAX + 1];
      u32 portid = skb ? NETLINK_CB(skb).portid : 0;
      ....
    }

    PVS-Studio Warnung: V595 Der Zeiger 'skb' wurde verwendet, bevor er gegen nullptr verifiziert wurde. Check lines: 949, 951. act_api.c 949 Hier ist

    alles einfach. Wenn der Zeiger 'skb' Null ist, treten Probleme auf. Der Zeiger wird in der ersten Zeile dereferenziert.

    Es ist zu beachten, dass der Analysator nicht schwört, weil er die Dereferenzierung eines nicht verifizierten Zeigers festgestellt hat. Es würde also zu viele Fehlalarme geben. Schließlich muss das Funktionsargument nicht immer 0 sein. Vielleicht wurde die Prüfung an einer anderen Stelle durchgeführt.

    Die Logik der Arbeit ist anders. PVS-Studio hält den Code für gefährlich, wenn der Zeiger dereferenziert und anschließend unten überprüft wird. Wenn der Zeiger markiert ist, wird angenommen, dass er 0 sein kann. Daher ist dies eine Gelegenheit, eine Warnung auszugeben.

    Dieses einfache Beispiel wurde aussortiert. Aber wir interessieren uns überhaupt nicht für ihn.

    Kommen wir nun zum komplexeren Fall von Optimierungen, die Compiler durchführen.
    static int podhd_try_init(struct usb_interface *interface,
            struct usb_line6_podhd *podhd)
    {
      int err;
      struct usb_line6 *line6 = &podhd->line6;
      if ((interface == NULL) || (podhd == NULL))
        return -ENODEV;
      ....
    }

    PVS-Studio Warnung: V595 Der Zeiger 'podhd' wurde verwendet, bevor er gegen nullptr verifiziert wurde. Check lines: 96, 98. podhd.c 96

    Dies ist ein Beispiel für einen Code, bei dem die Leute anfangen zu streiten und sagen, dass alles in Ordnung ist. Sie argumentieren so.

    Der Zeiger podhd sei NULL. Der Ausdruck & podhd-> line6 sieht hässlich aus. Aber es gibt keinen Fehler. Es besteht kein Speicherzugriff. Die Adresse eines der Klassenmitglieder wird einfach berechnet. Ja, der Wert des Zeigers 'line6' ist falsch. Es zeigt "nirgendwo" an. Dieser Zeiger wird jedoch nicht verwendet. Die falsche Adresse wurde berechnet, na und? Unten im Code gibt es eine Prüfung und wenn 'podhd', dann wird die Funktion ihre Arbeit beenden. Der Zeiger 'line6' wird nirgends verwendet, was bedeutet, dass in der Praxis kein Fehler auftritt.

    Meine Herren, Sie irren sich! Das kannst du sowieso nicht. Seien Sie nicht faul und korrigieren Sie einen solchen Code.

    Während der Optimierung argumentiert der Compiler wie folgt. Hier wird der Zeiger dereferenziert: podhd-> line6. Ja, der Programmierer weiß, was er tut. Der Zeiger hier ist also definitiv nicht Null. Großartig, denk dran.

    Dann stößt der Compiler auf eine Prüfung:
    if ((interface == NULL) || (podhd == NULL))
      return -ENODEV;

    Und optimiert es. Er betrachtet den Zeiger 'podhd' als nicht Null. Daher reduziert sich die Validierung auf:
    if ((interface == NULL))
      return -ENODEV;

    Wie im Fall von memset () besteht die zusätzliche Schwierigkeit darin, dass die fehlende Überprüfung in der Debug () - Version nicht angezeigt wird. Ein solcher Fehler ist sehr schwer zu suchen.

    Wenn Sie einen Nullzeiger an die Funktion übergeben, gibt die Funktion daher nicht den Status (-ENODEV) zurück, sondern setzt ihre Arbeit fort. Die Folgen können unvorhersehbar sein.

    Folgendes ist wichtig. Der Compiler kann wichtige Zeigerprüfungen aus schlecht geschriebenem Code entfernen. Das heißt Es gibt Funktionen, die nur Zeiger zu prüfen scheinen. Tatsächlich arbeiten sie mit einem Nullzeiger. Ich weiß nicht, ob es irgendwie verwendet werden kann. Meiner Meinung nach kann eine solche Situation jedoch als potenzielle Sicherheitslücke angesehen werden.

    Ein weiteres ähnliches Beispiel:
    int wpa_set_keys(struct vnt_private *pDevice, void *ctx,
         bool fcpfkernel) __must_hold(&pDevice->lock)
    {
      ....
      if (is_broadcast_ether_addr(¶m->addr[0]) ||
          (param->addr == NULL)) {
      ....
    }

    PVS-Studio-Warnung: V713 Der Zeigerparameter-> addr wurde im logischen Ausdruck verwendet, bevor er im selben logischen Ausdruck gegen nullptr verifiziert wurde. wpactl.c 333

    Der Compiler kann während der Optimierung den Scan auf Folgendes reduzieren:
    if (is_broadcast_ether_addr(¶m->addr[0]))

    Der Linux-Kernel ist groß. So wurden V595-Warnungen über 200 Stück ausgegeben. Zu meiner Schande war ich zu faul, sie anzuschauen und wählte nur ein Beispiel für den Artikel. Ich empfehle einem der Entwickler, den Rest der verdächtigen Orte zu erkunden. Ich liste sie auf: Linux-V595.txt .

    Ja, bei weitem nicht alle Warnungen weisen auf einen echten Fehler hin. Oft kann ein Zeiger nicht genau Null sein. Diese Liste ist jedoch eine Erkundung wert. Sie können mit ziemlicher Sicherheit ein paar Dutzend echte Fehler finden.

    Verdächtige Orte gefunden


    Möglicherweise enthalten nicht alle im Artikel beschriebenen Codefragmente einen echten Fehler. Sie sind jedoch äußerst misstrauisch und erfordern besondere Aufmerksamkeit für das Studium.

    Ungültige logische Bedingungen


    void b43legacy_phy_set_antenna_diversity(....)
    {
      ....
      if (phy->rev >= 2) {
        b43legacy_phy_write(
          dev, 0x0461, b43legacy_phy_read(dev, 0x0461) | 0x0010);
        ....
      } else if (phy->rev >= 6)
        b43legacy_phy_write(dev, 0x049B, 0x00DC);
      ....
    }

    PVS-Studio Warnung: V695-Bereichsüberschneidungen sind in bedingten Ausdrücken möglich. Beispiel: if (A <5) {...} else if (A <2) {...}. Check lines: 2147, 2162. phy.c 2162

    Die zweite Bedingung ist niemals erfüllt. Vereinfachen Sie aus Gründen der Übersichtlichkeit den Code:
    if ( A >= 2)
      X();
    else if ( A >= 6)
      Y();

    Wie Sie sehen, gibt es in der Variablen 'A' keinen solchen Wert, bei dem die Y () - Funktion aufgerufen wird.

    Betrachten Sie andere ähnliche Fälle. Sie brauchen keine Erklärung.
    static int __init scsi_debug_init(void)
    {
      ....
      if (scsi_debug_dev_size_mb >= 16)
        sdebug_heads = 32;
      else if (scsi_debug_dev_size_mb >= 256)
       sdebug_heads = 64;
      ....
    }

    PVS-Studio Warnung: V695-Bereichsüberschneidungen sind in bedingten Ausdrücken möglich. Beispiel: if (A <5) {...} else if (A <2) {...}. Überprüfen Sie die Zeilen: 3858, 3860. scsi_debug.c 3860

    static ssize_t ad5933_store(....)
    {
      ....
      /* 2x, 4x handling, see datasheet */
      if (val > 511)
        val = (val >> 1) | (1 << 9);
      else if (val > 1022)
        val = (val >> 2) | (3 << 9);
      ....
    }

    PVS-Studio Warnung: In bedingten Ausdrücken sind V695-Bereichsüberschneidungen möglich. Beispiel: if (A <5) {...} else if (A <2) {...}. Check lines: 439, 441. ad5933.c 441

    Es gibt einige ähnliche Situationen, die ich nicht angeben werde, um den Artikel nicht zu lang zu machen:
    • V695-Bereichsüberschneidungen sind in bedingten Ausdrücken möglich. Beispiel: if (A <5) {...} else if (A <2) {...}. Überprüfen Sie die Zeilen: 1417, 1422. bnx2i_hwi.c 1422
    • V695-Bereichsüberschneidungen sind in bedingten Ausdrücken möglich. Beispiel: if (A <5) {...} else if (A <2) {...}. Überprüfen Sie die Zeilen: 4815, 4831. stv090x.c 4831
    Betrachten Sie nun eine andere Art von verdächtigen Zuständen.
    static int dgap_parsefile(char **in)
    {
      ....
      int module_type = 0;
      ....
      module_type = dgap_gettok(in);
      if (module_type == 0 || module_type != PORTS ||
          module_type != MODEM) {
        pr_err("failed to set a type of module");
        return -1;
      }
      ....
    }

    PVS-Studio Warnung: V590 Prüfen Sie den 'module_type == 0 || module_type! = 68 'Ausdruck. Der Ausdruck ist zu groß oder enthält einen Druckfehler. dgap.c 6733

    Ich kenne den Code nicht und habe keine Ahnung, wie diese Prüfung aussehen soll. Daher werde ich auf eine Stellungnahme verzichten. Hier ist ein anderer ähnlicher Ort:
    • V590 Betrachten Sie die 'conc_type == 0 || conc_type! = 65 'Ausdruck. Der Ausdruck ist zu groß oder enthält einen Druckfehler. dgap.c 6692

    "Das Auge herausreißen"


    Während des Studiums der Analyzer-Meldungen bin ich auf die Funktion name_msi_vectors () gestoßen. Obwohl es kurz ist, möchte ich es überhaupt nicht lesen. Anscheinend enthält es deshalb eine sehr verdächtige Zeile.
    static void name_msi_vectors(struct ipr_ioa_cfg *ioa_cfg)
    {
      int vec_idx, n = sizeof(ioa_cfg->vectors_info[0].desc) - 1;
      for (vec_idx = 0; vec_idx < ioa_cfg->nvectors; vec_idx++) {
        snprintf(ioa_cfg->vectors_info[vec_idx].desc, n,
           "host%d-%d", ioa_cfg->host->host_no, vec_idx);
        ioa_cfg->vectors_info[vec_idx].
          desc[strlen(ioa_cfg->vectors_info[vec_idx].desc)] = 0;
      }
    }

    Warnung: V692 Ein unangemessener Versuch, ein Nullzeichen an eine Zeichenfolge anzuhängen. Um die Länge eines Strings durch die Funktion 'strlen' korrekt zu bestimmen, sollte an erster Stelle ein String verwendet werden, der mit einem Nullterminator endet. ipr.c 9409 Die

    letzte Zeile ist verdächtig:
    ioa_cfg->vectors_info[vec_idx].
      desc[strlen(ioa_cfg->vectors_info[vec_idx].desc)] = 0;

    Jetzt werde ich es vereinfachen und es wird sofort klar, dass hier etwas nicht stimmt:
    S[strlen(S)] = 0;

    Es hat keinen Sinn, so etwas zu tun. Null wird dort geschrieben, wo sie bereits ist. Es besteht der Verdacht, dass sie etwas anderes machen wollten.

    Endloses Warten


    static int ql_wait_for_drvr_lock(struct ql3_adapter *qdev)
    {
      int i = 0;
      while (i < 10) {
        if (i)
          ssleep(1);
        if (ql_sem_lock(qdev,
            QL_DRVR_SEM_MASK,
            (QL_RESOURCE_BITS_BASE_CODE | (qdev->mac_index)
             * 2) << 1)) {
          netdev_printk(KERN_DEBUG, qdev->ndev,
                  "driver lock acquired\n");
          return 1;
        }
      }
      netdev_err(qdev->ndev,
                 "Timed out waiting for driver lock...\n");
      return 0;
    }

    PVS-Studio Warnung: V654 Die Bedingung 'i <10' der Schleife ist immer wahr. qla3xxx.c 149

    Die Funktion versucht, den Treiber zu sperren. Wenn dies fehlschlägt, wartet sie 1 Sekunde und versucht es erneut. Insgesamt sollten 10 Versuche unternommen werden.

    Tatsächlich wird es jedoch unendlich viele Versuche geben. Der Grund ist, dass die Variable 'i' nirgendwo größer wird.

    Falsche Fehlerinformationen


    static int find_boot_record(struct NFTLrecord *nftl)
    {
      ....
      if ((ret = nftl_read_oob(mtd, block * nftl->EraseSize +
                               SECTORSIZE + 8, 8, &retlen,
                               (char *)&h1) < 0) ) {
        printk(KERN_WARNING "ANAND header found at 0x%x in mtd%d, "
               "but OOB data read failed (err %d)\n",
               block * nftl->EraseSize, nftl->mbd.mtd->index, ret);
        continue;
      ....
    }

    PVS-Studio Warnung: V593 Überprüfen Sie den Ausdruck der Art 'A = B <C'. Der Ausdruck wird wie folgt berechnet: 'A = (B <C)'. nftlmount.c 92

    Wenn ein Fehler auftritt, sollte die Funktion Informationen darüber ausgeben . Einschließlich sollte Fehlercode gedruckt werden. Tatsächlich wird jedoch (err 0) oder (err 1) gedruckt, nicht der tatsächliche Fehlercode.

    Der Grund ist, dass der Programmierer über die Prioritäten der Operation verwirrt ist. Am Anfang wollte er das Ergebnis der Funktion nftl_read_oob () in die Variable 'ret' setzen. Dann möchte er diese Variable mit 0 vergleichen. Wenn (ret <0), müssen Sie eine Fehlermeldung drucken.

    In der Tat funktioniert nicht alles so. Zu Beginn wird das Ergebnis der Funktion nftl_read_oob () mit 0 verglichen. Das Ergebnis des Vergleichs ist der Wert 0 oder 1. Dieser Wert wird in die Variable 'ret' gestellt.

    Wenn also die Funktion nftl_read_oob () einen negativen Wert zurückgibt, ist ret == 1. Es wird eine Meldung angezeigt, die jedoch falsch ist.

    Die Bedingung zeigt, dass zusätzliche Klammern verwendet werden. Es ist nicht bekannt, ob sie geschrieben wurden, um die Compiler-Warnung bezüglich der Zuweisung innerhalb von 'if' zu unterdrücken oder um die Reihenfolge der Operationen explizit anzugeben. Im zweiten Fall handelt es sich um einen Tippfehler - die schließende Klammer ist nicht an ihrer Stelle. Es wird so richtig sein:
    if ((ret = nftl_read_oob(mtd, block * nftl->EraseSize +
                             SECTORSIZE + 8, 8, &retlen,
                             (char *)&h1)) < 0 ) {

    Tippfehler Verdacht


    int wl12xx_acx_config_hangover(struct wl1271 *wl)
    {
      ....
      acx->recover_time = cpu_to_le32(conf->recover_time);
      acx->hangover_period = conf->hangover_period;
      acx->dynamic_mode = conf->dynamic_mode;
      acx->early_termination_mode = conf->early_termination_mode;
      acx->max_period = conf->max_period;
      acx->min_period = conf->min_period;
      acx->increase_delta = conf->increase_delta;
      acx->decrease_delta = conf->decrease_delta;
      acx->quiet_time = conf->quiet_time;
      acx->increase_time = conf->increase_time;
      acx->window_size = acx->window_size;         <<<---
      ....
    }

    PVS-Studio Warnung: V570 Die Variable 'acx-> window_size' ist sich selbst zugeordnet. acx.c 1728

    Überall werden die Felder einer Struktur in die Felder einer anderen Struktur kopiert. Und nur an einer Stelle steht plötzlich:
    acx->window_size = acx->window_size;

    Das ist ein Fehler? Oder hast du das gedacht? Ich kann keine Antwort geben.

    Verdächtige Oktalzahl


    static const struct XGI330_LCDDataDesStruct2  XGI_LVDSNoScalingDesData[] = {
      {0,  648,  448,  405,  96, 2}, /* 00 (320x200,320x400,
                                            640x200,640x400) */
      {0,  648,  448,  355,  96, 2}, /* 01 (320x350,640x350) */
      {0,  648,  448,  405,  96, 2}, /* 02 (360x400,720x400) */
      {0,  648,  448,  355,  96, 2}, /* 03 (720x350) */
      {0,  648,    1,  483,  96, 2}, /* 04 (640x480x60Hz) */
      {0,  840,  627,  600, 128, 4}, /* 05 (800x600x60Hz) */
      {0, 1048,  805,  770, 136, 6}, /* 06 (1024x768x60Hz) */
      {0, 1328,    0, 1025, 112, 3}, /* 07 (1280x1024x60Hz) */
      {0, 1438,    0, 1051, 112, 3}, /* 08 (1400x1050x60Hz)*/
      {0, 1664,    0, 1201, 192, 3}, /* 09 (1600x1200x60Hz) */
      {0, 1328,    0, 0771, 112, 6}  /* 0A (1280x768x60Hz) */
                      ^^^^
                      ^^^^
    };

    PVS-Studio Warnung: V536 Beachten Sie, dass der verwendete Konstantenwert durch eine Oktalform dargestellt wird. Okt: 0771, Dez: 505. vb_table.h 1379

    Alle Zahlen in dieser Struktur werden im Dezimalformat angegeben. Und plötzlich gibt es eine Oktalzahl: 0771. Der Analysator hat dies alarmiert. Und ich auch.

    Es besteht der Verdacht, dass diese Null geschrieben wird, um die Spalte schön auszurichten. Dann ist das aber eindeutig ein falscher Wert.

    Verdächtige Leitung


    static void sig_ind(PLCI *plci)
    {
      ....
      byte SS_Ind[] =
        "\x05\x02\x00\x02\x00\x00"; /* Hold_Ind struct*/
      byte CF_Ind[] =
        "\x09\x02\x00\x06\x00\x00\x00\x00\x00\x00";
      byte Interr_Err_Ind[] =
        "\x0a\x02\x00\x07\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00";
      byte CONF_Ind[] =
        "\x09\x16\x00\x06\x00\x00\0x00\0x00\0x00\0x00";
                                  ^^^^^^^^^^^^^^^^^^^
      ....
    }

    PVS-Studio Warnung: V638 Innerhalb eines Strings befindet sich eine Terminal-Null. Die Zeichen '\ 0x00' wurden gefunden. Vermutlich gemeint: '\ x00'. message.c 4883

    Arrays enthalten einige magische Werte. Der Verdacht ruft den Inhalt des Arrays CONF_Ind [] auf. Es wird abwechselnd mit Nullzeichen und dem Text "x00" angezeigt. Ich denke, das ist ein Tippfehler, und in der Tat sollte es so geschrieben werden:
    byte CONF_Ind[] =
      "\x09\x16\x00\x06\x00\x00\x00\x00\x00\x00";

    Das heißt '0' vor 'x' ist überflüssig und wird versehentlich geschrieben. Daher wird „x00“ als Text und nicht als Zeichencode interpretiert.

    Verdächtige Code-Formatierung


    static int grip_xt_read_packet(....)
    {
      ....
      if ((u ^ v) & 1) {
        buf = (buf << 1) | (u >> 1);
        t = strobe;
        i++;
      } else
      if ((((u ^ v) & (v ^ w)) >> 1) & ~(u | v | w) & 1) {
      ....
    }

    PVS-Studio Warnung: V705 Es ist möglich, dass der 'else'-Block vergessen oder auskommentiert wurde, wodurch die Funktionslogik des Programms geändert wird. grip.c 152

    Ich glaube nicht, dass hier ein Fehler vorliegt . Der Code ist jedoch sehr schlecht formatiert. Deshalb zitiere ich es im Artikel. Es ist besser, es noch einmal zu überprüfen.

    Undefiniertes Verhalten bei Schervorgängen


    static s32 snto32(__u32 value, unsigned n)
    {
      switch (n) {
      case 8:  return ((__s8)value);
      case 16: return ((__s16)value);
      case 32: return ((__s32)value);
      }
      return value & (1 << (n - 1)) ? value | (-1 << n) : value;
    }

    Warnung PVS-Studio: V610 Undefiniertes Verhalten. Überprüfen Sie den Schichtführer '<<. Der linke Operand '-1' ist negativ. hid-core.c 1016 Das

    Verschieben negativer Werte führt zu undefiniertem Verhalten. Ich habe darüber viele Male geschrieben und werde nicht näher darauf eingehen. Für diejenigen, die nicht wissen, was los ist, schlage ich vor, sich mit dem Artikel " Kenne die Furt nicht, gehe nicht ins Wasser. Teil 3 (über Schichten) " vertraut zu machen .

    Ich sehe einen Einwand im Geiste voraus: "Aber es funktioniert!" Vielleicht klappt es ja. Aber ich denke, der Linux-Kernel ist kein Ort, an dem man sich auf diesen Ansatz verlassen kann. Der Code ist besser umzuschreiben.

    Da es viele solcher Verschiebungen gibt, habe ich sie in einer separaten Datei gesammelt: Linux-V610.txt .

    Verwechslung mit Aufzählung


    Es gibt zwei solche Aufzählungen:
    enum iscsi_param {
      ....
      ISCSI_PARAM_CONN_PORT,
      ISCSI_PARAM_CONN_ADDRESS,        <<<<----
      ....
    };
    enum iscsi_host_param {
      ISCSI_HOST_PARAM_HWADDRESS,
      ISCSI_HOST_PARAM_INITIATOR_NAME,
      ISCSI_HOST_PARAM_NETDEV_NAME,
      ISCSI_HOST_PARAM_IPADDRESS,       <<<<----
      ISCSI_HOST_PARAM_PORT_STATE,
      ISCSI_HOST_PARAM_PORT_SPEED,
      ISCSI_HOST_PARAM_MAX,
    };

    Beachten Sie die Konstanten ISCSI_PARAM_CONN_ADDRESS und ISCSI_HOST_PARAM_IPADDRESS. Ihre Namen sind ähnlich. Anscheinend war dies der Grund für die Verwirrung.

    Betrachten Sie das Code-Snippet:
    int iscsi_conn_get_addr_param(
      struct sockaddr_storage *addr,
      enum iscsi_param param, char *buf)
    {
      ....
      switch (param) {
      case ISCSI_PARAM_CONN_ADDRESS:
      case ISCSI_HOST_PARAM_IPADDRESS:        <<<<----
      ....
      case ISCSI_PARAM_CONN_PORT:
      case ISCSI_PARAM_LOCAL_PORT:
      ....
      default:
        return -EINVAL;
      }
      return len;
    }

    PVS-Studio Warnung: V556 Die Werte verschiedener Aufzählungstypen werden verglichen: switch (ENUM_TYPE_A) {case ENUM_TYPE_B: ...}. libiscsi.c 3501 Die

    Konstante ISCSI_HOST_PARAM_IPADDRESS gilt nicht für die Aufzählung iscsi_param. Dies ist höchstwahrscheinlich ein Tippfehler, und die Konstante ISCSI_PARAM_CONN_ADDRESS sollte verwendet werden.

    Ähnliche Warnungen von PVS-Studio:
    • V556 Die Werte verschiedener Aufzählungstypen werden verglichen: switch (ENUM_TYPE_A) {case ENUM_TYPE_B: ...}. svm.c 1360
    • V556 Die Werte verschiedener Aufzählungstypen werden verglichen: switch (ENUM_TYPE_A) {case ENUM_TYPE_B: ...}. vmx.c 2690
    • V556 Die Werte verschiedener Aufzählungstypen werden verglichen: switch (ENUM_TYPE_A) {case ENUM_TYPE_B: ...}. request.c 2842
    • V556 Die Werte verschiedener Aufzählungstypen werden verglichen: switch (ENUM_TYPE_A) {case ENUM_TYPE_B: ...}. request.c 2868

    Seltsamer Zyklus


    Hier kann ich das Codefragment nicht anzeigen. Das Fragment ist groß und ich weiß nicht, wie ich es verkleinern und schön formatieren soll. Deshalb werde ich Pseudocode schreiben.
    void pvr2_encoder_cmd ()
    {
      do {
        ....
        if (A) break;
        ....
        if (B) break;
        ....
        if (C) continue;
        ....
        if (E) break;
        ....
      } while(0);
    }

    Der Zyklus wird einmal ausgeführt. Es besteht der Verdacht, dass ein solches Design erstellt wurde, um auf den goto-Operator zu verzichten. Wenn ein Fehler aufgetreten ist, wird die Anweisung 'break' aufgerufen und die Anweisungen nach der Schleife werden ausgeführt.

    Es verwirrt mich, dass an einer Stelle der Operator 'continue' steht und nicht 'break'. In diesem Fall funktioniert der Operator 'continue' wie 'break'. Ich werde es erklären.

    Der Standard sagt

    Folgendes : §6.6.2 im Standard: „Die continue-Anweisung (...) bewirkt, dass die Steuerung an den Schleifenfortsetzungsabschnitt der kleinsten einschließenden Iterationsanweisung übergeben wird, dh an das Ende der Schleife. ”(Nicht zum Anfang.)

    Nach dem Aufrufen der 'continue'-Anweisung wird die Bedingung (0) überprüft und der Zyklus wird beendet, da die Bedingung falsch ist.

    Vielleicht 2 Möglichkeiten.
    1. Der Code ist korrekt. Die 'continue'-Anweisung muss die Schleife unterbrechen. Dann empfehle ich, es aus Konsistenzgründen durch 'break' zu ersetzen, damit Entwickler, die in Zukunft mit diesem Code arbeiten, nicht verwirrt werden.
    2. Die 'continue'-Anweisung sollte, wie vom Programmierer beabsichtigt, einen Zyklus fortsetzen. Dann ist der Code falsch und sollte neu geschrieben werden.

    Fehler beim Kopieren und Einfügen


    void dm_change_dynamic_initgain_thresh(
      struct net_device *dev, u32 dm_type, u32 dm_value)
    {
      ....
      if (dm_type == DIG_TYPE_THRESH_HIGH)
      {
        dm_digtable.rssi_high_thresh = dm_value;
      }
      else if (dm_type == DIG_TYPE_THRESH_LOW)
      {
        dm_digtable.rssi_low_thresh = dm_value;
      }
      else if (dm_type == DIG_TYPE_THRESH_HIGHPWR_HIGH)      <<--
      {                                                      <<--
        dm_digtable.rssi_high_power_highthresh = dm_value;   <<--
      }                                                      <<--
      else if (dm_type == DIG_TYPE_THRESH_HIGHPWR_HIGH)      <<--
      {                                                      <<--
        dm_digtable.rssi_high_power_highthresh = dm_value;   <<--
      }                                                      <<--
      ....
    }

    PVS-Studio-Warnung: V517 Die Verwendung des Musters 'if (A) {...} else if (A) {...}' wurde erkannt. Es ist wahrscheinlich, dass ein logischer Fehler vorliegt. Check lines: 1755, 1759. r8192U_dm.c 1755

    Der Code wurde mit Copy-Paste geschrieben und hat vergessen, ihn in einem Textblock zu ersetzen:
    • DIG_TYPE_THRESH_HIGHPWR_HIGH bei DIG_TYPE_THRESH_HIGHPWR_LOW
    • rssi_high_power_highthresh auf rssi_high_power_lowthresh
    Außerdem bitte ich die Entwickler, hier nachzuschauen:
    • V517 Die Verwendung des Musters 'if (A) {...} else if (A) {...}' wurde erkannt. Es ist wahrscheinlich, dass ein logischer Fehler vorliegt. Überprüfen Sie die Zeilen: 1670, 1672. rtl_dm.c 1670
    • V517 Die Verwendung des Musters 'if (A) {...} else if (A) {...}' wurde erkannt. Es ist wahrscheinlich, dass ein logischer Fehler vorliegt. Zeilen prüfen: 530, 533. ioctl.c 530

    Reinitialisierung


    Es gibt merkwürdige Stellen, an denen einer Variablen zwei Mal hintereinander unterschiedliche Werte zugewiesen werden. Ich denke, es lohnt sich, diese Codeabschnitte zu lesen.
    static int saa7164_vbi_fmt(struct file *file, void *priv,
                               struct v4l2_format *f)
    {
      /* ntsc */
      f->fmt.vbi.samples_per_line = 1600;           <<<<----
      f->fmt.vbi.samples_per_line = 1440;           <<<<----
      f->fmt.vbi.sampling_rate = 27000000;
      f->fmt.vbi.sample_format = V4L2_PIX_FMT_GREY;
      f->fmt.vbi.offset = 0;
      f->fmt.vbi.flags = 0;
      f->fmt.vbi.start[0] = 10;
      f->fmt.vbi.count[0] = 18;
      f->fmt.vbi.start[1] = 263 + 10 + 1;
      f->fmt.vbi.count[1] = 18;
      return 0;
    }

    PVS-Studio Warnung: V519 Die Variable 'f-> fmt.vbi.samples_per_line' wird zweimal nacheinander mit Werten belegt. Vielleicht ist das ein Fehler. Überprüfen Sie die Zeilen: 1001, 1002. saa7164-vbi.c 1002
    static int saa7164_vbi_buffers_alloc(struct saa7164_port *port)
    {
      ....
      /* Init and establish defaults */
      params->samplesperline = 1440;
      params->numberoflines = 12;                           <<<<----
      params->numberoflines = 18;                           <<<<----
      params->pitch = 1600;                                 <<<<----
      params->pitch = 1440;                                 <<<<----
      params->numpagetables = 2 +
        ((params->numberoflines * params->pitch) / PAGE_SIZE);
      params->bitspersample = 8;
       ....
    }

    Warnungen:
    • V519 Der Variablen 'params-> numberoflines' werden nacheinander zweimal Werte zugewiesen. Vielleicht ist das ein Fehler. Zeilen überprüfen: 118, 119. saa7164-vbi.c 119
    • V519 The 'params->pitch' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 120, 121. saa7164-vbi.c 121


    Заключение


    In jedem großen Projekt finden Sie einige Fehler. Der Linux-Kernel war keine Ausnahme. Einmalige Codeüberprüfungen durch einen statischen Analysator sind jedoch die falsche Methode. Ja, sie erlauben es Ihnen, einen solchen Werbeartikel zu schreiben, aber sie bringen wenig Nutzen für das Projekt.

    Wenn Sie die statische Analyse regelmäßig verwenden, sparen Sie eine Menge Zeit, indem Sie eine Reihe von Fehlern im frühesten Stadium ihres Auftretens erkennen. Schützen Sie Ihr Projekt mit einem statischen Analysegerät vor Fehlern!

    Linux und PVS-Studio

    Ich empfehle jedem, PVS-Studio bei seinen Projekten auszuprobieren . Der Analyzer läuft in einer Windows-Umgebung. Wenn jemand es bei der Entwicklung großer Linux-Anwendungen verwenden möchte, dann schreiben Sie unsund wir besprechen mögliche Optionen für den Abschluss eines Vertrags zur Anpassung von PVS-Studio an Ihre Projekte und Aufgaben.

    Dieser Artikel ist in englischer Sprache.


    Wenn Sie diesen Artikel mit einem englischsprachigen Publikum teilen möchten, verwenden Sie bitte den Link zur Übersetzung: Andrey Karpov. PVS-Studio befasst sich mit Linux Insides (3.18.1) .

    Haben Sie den Artikel gelesen und eine Frage?
    Oft werden unseren Artikeln die gleichen Fragen gestellt. Hier haben wir Antworten auf diese Fragen gesammelt: Antworten auf Fragen von Lesern von Artikeln über PVS-Studio und CppCat, Version 2014 . Bitte beachten Sie die Liste.

    Jetzt auch beliebt: