PVS-Studioを使用したIntel GalileoのオープンソースUEFIの確認

ファームウェアの開発は、エキゾチックなアーキテクチャのアセンブラーではなく、i386 / amd64のCで行われる場合でも、かなり複雑な問題であり、ターゲットハードウェアプラットフォームに障害が発生した場合でもエラーのコストが非常に高くなる可能性があるため、さまざまなエラー防止技術を使用してください開発の非常に初期の段階で-必要性。



残念ながら、UEFIファームウェアの場合、正式な検証またはMISRA Cの使用のみを夢見ることができます(一方、ファームウェアの開発に数年とプロジェクト予算の50%を費やすことを望む人はほとんどいません)。したがって、今日は静的分析、または、 Habréで人気のあるPVS-Studioスタティックアナライザー。IntelGalileoのオープンUEFIコードでエラーの検出を試みます。



catの下でチェックの結果を謙虚にお願いします。



環境設定

キャプテンエビデンスが私に言うように、コードを分析するには、アナライザー、コード自体、およびそのためのアセンブリ環境が必要です。



アナライザーはその開発者のWebサイトにあります。インストール後、短時間キーを提供するように彼に手紙を書きます。第1レベルの警告(これは試用版で行われたものです)だけでなく、一般的なことすべてを見ることができます。



ファームウェアコードは、 Quark BSPの一部であり、Apple製品を除くすべての最新のUEFI実装と同様に、 EDK 2010.SR1に基づいています。



EDKには独自のビルドシステムがあるため、収集したコードを確認するには、PVS-Studioスタンドアロンを使用します。 アセンブリ用にQuark_EDKIIパッケージを準備する方法- このドキュメントを読んで 、ここでは詳しく説明しません。



アナライザー開始

PVS-Studio Standaloneを起動し、[ファイルの分析...]ボタンをクリックすると、[コンパイラの監視]ウィンドウが開き、[監視の開始]ボタンのみをクリックします。 ここで、Quark_EDKIIのあるフォルダーでコンソールを開き、 quarkbuild -r32 S QuarkPlatformコマンドを実行して、リリースバージョンのファームウェアをビルドします。 アセンブリが終了するのを待っています。コンパイラ監視ウィンドウで、検出されたコンパイラ呼び出しの数の増加を確認します。アセンブリが完了したら、監視の停止ボタンをクリックし、分析が完了するのを待ちます。



分析結果

Quark_EDKII_v1.1.0の現在のバージョンでは、アナライザーは第1レベルの96個の警告、第2レベルの100個、第3レベルの63個の警告を表示します(デフォルト設定、つまり一般分析のみが選択されている場合)。 警告番号で並べ替えて、エラーの検索に進みます。



警告 :V521 '、'演算子を使用したこのような式は危険です。 式が正しいことを確認してください。

ファイル :quarkplatformpkg \ pci \ dxe \ pcihostbridge \ pcihostbridge.c、181、272

コード
for (TotalRootBridgeFound = 0, IioResourceMapEntry = 0; TotalRootBridgeFound < HostBridge->RootBridgeCount, IioResourceMapEntry < MaxIIO; IioResourceMapEntry++) {
      
      



コメント :条件内のコンマ演算子の誤用。 この演算子の優先順位は最も低く、両方のオペランドの値を計算しますが、正しい演算子の値を使用することを思い出してください。 この場合、条件はIioResourceMapEntry <MaxIIOと完全に類似しており、TotalRootBridgeFound <HostBridge-> RootBridgeCountチェックは満たされますが、サイクルの継続または中断には影響しません。

修正案 :条件内のコンマを&&に置き換えます。



警告 :V524「AllocateRuntimePages」関数の本体が「AllocatePages」関数の本体と完全に同等であることは奇妙です。

ファイル :mdepkg \ library \ smmmemoryallocationlib \ memoryallocationlib.c、208 ff

コード
 /** Allocates one or more 4KB pages of type EfiBootServicesData. Allocates the number of 4KB pages of type EfiBootServicesData and returns a pointer to the allocated buffer. The buffer returned is aligned on a 4KB boundary. If Pages is 0, then NULL is returned. If there is not enough memory remaining to satisfy the request, then NULL is returned. @param Pages The number of 4 KB pages to allocate. @return A pointer to the allocated buffer or NULL if allocation fails. **/ VOID * EFIAPI AllocatePages ( IN UINTN Pages ) { return InternalAllocatePages (EfiRuntimeServicesData, Pages); }
      
      



コメント :コードはコメントと矛盾し、暗黙のEfiBootServicesDataの代わりにEfiRuntimeServicesDataのようなメモリを割り当てます。 それらの違いは、 BDSフェーズの終了時に2番目のタイプのメモリが自動的に解放されることです。最初のタイプのメモリは、上記のフェーズが終了する前にFreeMemへの明示的な呼び出しによって解放する必要があります。 その結果、一見小さなエラーでも、完全に理解できないメモリリークや、OSで使用可能なアドレス空間の断片化につながる可能性があります。

推奨される修正 :このファイルのすべての非ランタイム関数で、メモリタイプをEfiBootServicesDataに置き換えます。



警告 :V524「OhciSetLsThreshold」関数の本体が「OhciSetPeriodicStart」関数の本体と完全に同等であることは奇妙です。

ファイル :quarksocpkg \ quarksouthcluster \ usb \ ohci \ pei \ ohcireg.c、1010、1015およびquarksocpkg \ quarksouthcluster \ usb \ ohci \ dxe \ ohcireg.c、1010、1040

コード
 EFI_STATUS OhciSetLsThreshold ( IN USB_OHCI_HC_DEV *Ohc, IN UINT32 Value ) { EFI_STATUS Status; Status = OhciSetOperationalReg (Ohc->PciIo, HC_PERIODIC_START, &Value); return Status; }
      
      



コメント :別のコピーペーストの被害者。今回は、HC_LS_THREASHOLDの代わりにHC_PERIODIC_STARTビットが設定され、チェックされます。

修正案 :ビットを正しいものに置き換えます。



警告 :V528 'char'型へのポインターが '\ 0'値と比較されるのは奇妙です。 おそらく意味:* MatchLang!= '\ 0'。

ファイル :quarkplatformpkg \ platform \ dxe \ smbiosmiscdxe \ miscnumberofinstallablelanguagesfunction.c、95

コード
 for (MatchLang = Languages, (*Offset) = 0; MatchLang != '\0'; (*Offset)++) { // // Seek to the end of current match language. // for (EndMatchLang = MatchLang; *EndMatchLang != '\0' && *EndMatchLang != ';'; EndMatchLang++); if ((EndMatchLang == MatchLang + CompareLength) && AsciiStrnCmp(MatchLang, BestLanguage, CompareLength) == 0) { // // Find the current best Language in the supported languages // break; } // // best language match be in the supported language. // ASSERT (*EndMatchLang == ';'); MatchLang = EndMatchLang + 1; }
      
      



コメント :参照されていないポインターをチェックする際のエラーの結果、サイクルは無限になり、ループの発生を防ぐためにその内部のみが見つかりました。

推奨される修正 :欠落している逆参照を追加します。



警告 :V535変数 'Index'は、このループと外側のループに使用されています。

ファイル :mdemodulepkg \ core \ pismmcore \ dispatcher.c、1233、1269、1316

コード
 for (Index = 0; Index < HandleCount; Index++) { FvHandle = HandleBuffer[Index]; ... for (Index = 0; Index < sizeof (mSmmFileTypes)/sizeof (EFI_FV_FILETYPE); Index++) { ... } ... for (Index = 0; Index < AprioriEntryCount; Index++) { ... } }
      
      



コメント :成功した環境の組み合わせのおかげでのみ機能するコードの例。 外側のループのHandleCountは、ほとんど常に1に等しく、mSmmFileTypes配列には、現時点でちょうど1つの要素もあり、AprioriEntryCountは1以上であるため、外側のループは終了します。 まったく異なる動作が暗示されていることは明らかですが、コピー&ペーストにはこの問題に関する独自の意見があります。

推奨される修正 :各サイクルに独立したカウンターを導入します。



警告 :V547式 '(0)>(1-Dtr1.field.tCMD)'は常にfalseです。 符号なしの型の値が<0になることはありません。

ファイル :quarksocpkg \ quarknorthcluster \ memoryinit \ pei \ meminit.c、483、487

コード
 #define MMAX(a,b) ((a)>(b)?(a):(b)) ... #pragma pack(1) typedef union { uint32_t raw; struct { ... uint32_t tCMD :2; /**< bit [5:4] Command transport duration */ ... } field; } RegDTR1; /**< DRAM Timing Register 1 */ #pragma pack() ... if (mrc_params->ddr_speed == DDRFREQ_800) { Dtr3.field.tXP = MMAX(0, 1 - Dtr1.field.tCMD); } else { Dtr3.field.tXP = MMAX(0, 2 - Dtr1.field.tCMD); }
      
      



コメント :最も単純なマクロと自動型キャストの取り消し。 なぜなら tCMDはuint32_t型のビットフィールドであるため、0> 1-tCMDの条件では、両方の部分が自動的にuint32_tにキャストされ、tCMD値に対してfalseになります。

修正案
 if (mrc_params->ddr_speed == DDRFREQ_800) { Dtr3.field.tXP = Dtr1.field.tCMD > 0 ? 0 : 1 ; } else { Dtr3.field.tXP = Dtr1.field.tCMD > 1 ? 0 : 2 - Dtr1.field.tCMD; }
      
      





警告 :V547式 'PollCount> =((1000 * 1000)/ 25)'は常にfalseです。 符号なしchar型の値の範囲:[0、255]。

ファイル :quarksocpkg \ quarksouthcluster \ i2c \ common \ i2ccommon.c、297

コード
 UINT8 PollCount; ... do { Data = *((volatile UINT32 *) (UINTN)(Addr)); if ((Data & I2C_REG_RAW_INTR_STAT_TX_ABRT) != 0) { Status = EFI_ABORTED; break; } if ((Data & I2C_REG_RAW_INTR_STAT_TX_OVER) != 0) { Status = EFI_DEVICE_ERROR; break; } if ((Data & I2C_REG_RAW_INTR_STAT_RX_OVER) != 0) { Status = EFI_DEVICE_ERROR; break; } if ((Data & I2C_REG_RAW_INTR_STAT_STOP_DET) != 0) { Status = EFI_SUCCESS; break; } MicroSecondDelay(TI2C_POLL); PollCount++; if (PollCount >= MAX_STOP_DET_POLL_COUNT) { Status = EFI_TIMEOUT; break; } } while (TRUE);
      
      



コメント :マクロMAX_STOP_DET_POLL_COUNTは40,000で展開し、PollCountは255を超えることはできません。その結果、無限ループが発生する可能性があります。

推奨される修正 :PollCountタイプをUINT32に変更します。



警告 :V560条件式の一部は常にtrue:(0x00040000)です。

ファイル :quarksocpkg \ quarknorthcluster \ library \ intelqnclib \ pciexpress.c、370

コード
 if ((QNCMmPci32 (0, Bus, Device, Function, (CapOffset + PCIE_LINK_CAP_OFFSET)) && B_QNC_PCIE_LCAP_CPM) != B_QNC_PCIE_LCAP_CPM) { return; }
      
      



コメント :ビット単位のANDの代わりに、式に論理的に入り込み、その結果、チェックは役に立たなくなりました。

修正案
 if ((QNCMmPci32 (0, Bus, Device, Function, (CapOffset + PCIE_LINK_CAP_OFFSET)) & B_QNC_PCIE_LCAP_CPM) != B_QNC_PCIE_LCAP_CPM) { return; }
      
      





警告 :V560条件式の一部は常に真です:0x0FFFFF000。

ファイル :quarksocpkg \ quarknorthcluster \ library \ intelqnclib \ intelqnclib.c、378

コード
 return QNCPortRead(QUARK_NC_HOST_BRIDGE_SB_PORT_ID, QUARK_NC_HOST_BRIDGE_HMBOUND_REG) && HMBOUND_MASK;
      
      



コメント :前の場合と同じ、さらに悪いことに、今回は戻り値が影響を受けました

修正案
 return QNCPortRead(QUARK_NC_HOST_BRIDGE_SB_PORT_ID, QUARK_NC_HOST_BRIDGE_HMBOUND_REG) & HMBOUND_MASK;
      
      





警告 :V560条件式の一部は常に真です:0x00400。

ファイル :quarksocpkg \ quarksouthcluster \ usb \ ohci \ pei \ ohcireg.c、1065およびquarksocpkg \ quarksouthcluster \ usb \ ohci \ dxe \ ohcireg.c、1070

コード
 if (Field & (RH_DEV_REMOVABLE || RH_PORT_PWR_CTRL_MASK)) {
      
      



コメント :今回は運が悪いOR。

修正案
 if (Field & (RH_DEV_REMOVABLE | RH_PORT_PWR_CTRL_MASK)) {
      
      





警告 :V649同一の条件式を持つ2つの「if」ステートメントがあります。 最初の「if」ステートメントには、関数の戻り値が含まれます。 これは、2番目の「if」ステートメントが無意味であることを意味します。

ファイル :s:\ quarkplatformpkg \ platform \ dxe \ smbiosmiscdxe \ miscsystemmanufacturerfunction.c、155

コード
  SerialNumStrLen = StrLen(SerialNumberPtr); if (SerialNumStrLen > SMBIOS_STRING_MAX_LENGTH) { return EFI_UNSUPPORTED; } ... SKUNumStrLen = StrLen(SKUNumberPtr); if (SerialNumStrLen > SMBIOS_STRING_MAX_LENGTH) { return EFI_UNSUPPORTED; } ... FamilyStrLen = StrLen(FamilyPtr); if (SerialNumStrLen > SMBIOS_STRING_MAX_LENGTH) { return EFI_UNSUPPORTED; }
      
      



コメント :間違ったコピーペーストが再び失敗しました。 1つの値を取得し、別の値を確認します。関数の理解できない動作があります。

修正案
  SerialNumStrLen = StrLen(SerialNumberPtr); if (SerialNumStrLen > SMBIOS_STRING_MAX_LENGTH) { return EFI_UNSUPPORTED; } ... SKUNumStrLen = StrLen(SKUNumberPtr); if (SKUNumStrLen > SMBIOS_STRING_MAX_LENGTH) { return EFI_UNSUPPORTED; } ... FamilyStrLen = StrLen(FamilyPtr); if (FamilyStrLen > SMBIOS_STRING_MAX_LENGTH) { return EFI_UNSUPPORTED; }
      
      





おわりに

シフト演算子の危険な使用、同じ変数の再割り当て、リテラルおよび整数変数のポインターへの変換などの問題に注意を払わず、明らかにエラーのあるコードのみを選択しようとしました。エラーがありますが、それでもリストはかなり印象的でした。 デスクトップマザーボードのプロジェクトはこれよりも平均で4〜5倍大きく(この例では800でなく、モニタリングウィンドウのカウンターによる約4000のコンパイラー呼び出し)、同じ典型的なエラーが発生します。



残念ながら、IntelはまだQuark_EDKIIのソースコードをGitHubに投稿していないため、このプロジェクトのプルリクエストをまだ誰にも送信していません。 おそらく、 Izardは、Intelの誰がプロジェクトの責任者であり、いつかバグを修正できるように誰をリンクに入れるべきかを知っているでしょう。



読者の注意に感謝し、PVS-Studioの開発者に有用なプログラムとテストキーを提供してくれました。



All Articles