Skip to content

Commit

Permalink
FloatSafeExit: CodeQL port of C28162 (#147)
Browse files Browse the repository at this point in the history
Port of C28162
  • Loading branch information
jacob-ronstadt authored Aug 23, 2024
1 parent c8369c9 commit 786fda0
Show file tree
Hide file tree
Showing 5 changed files with 669 additions and 0 deletions.
69 changes: 69 additions & 0 deletions src/drivers/general/queries/FloatSafeExit/FloatSafeExit.qhelp
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
<qhelp>
<overview>
<p>
Exiting while holding the right to use floating-point hardware
</p>
</overview>
<recommendation>
<p>
The _Kernel_float_restored_ annotation was used to release the right to use floating point, but a path through the function was detected where no function known to perform that operation was successfully called. This warning might indicate that a conditional (_When_) annotation is needed, or it might indicate a coding error.
</p>
</recommendation>
<example>
<p>
Example of function with multiple issues
</p>
<sample language="c"> <![CDATA[
void func(){
float f = 0.0f;
int some_condition = 1;
KFLOATING_SAVE saveData;
NTSTATUS status;
if (some_condition)
{
status = KeSaveFloatingPointState(&saveData);
if (!NT_SUCCESS(status))
{
return;
}
for (int i = 0; i < 100; i++)
{
f = f + 1;
}
// doesn't restore the floating point state
}
else
{
status = KeSaveFloatingPointState(&saveData);
if (!NT_SUCCESS(status))
{
return;
}
for (int i = 0; i < 100; i++)
{
f = f + 1.0f;
}
// doesn't check the return value of KeRestoreFloatingPointState
status = KeRestoreFloatingPointState(&saveData);
}
}
}]]>

</example>
<semmleNotes>
<p>
</p>
</semmleNotes>
<references>
<li>
<a href="https://learn.microsoft.com/en-us/windows-hardware/drivers/devtest/28162-exiting-while-holding-right-to-use-floating-hardware">
Warning C28162
</a>
</li>
</references>
</qhelp>
108 changes: 108 additions & 0 deletions src/drivers/general/queries/FloatSafeExit/FloatSafeExit.ql
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.
/**
* @id cpp/drivers/kmdf/float-safe-exit
* @kind problem
* @name Float Safe Exit
* @description Exiting while holding the right to use floating-point hardware
* @platform Desktop
* @feature.area Multiple
* @impact Insecure Coding Practice
* @repro.text
* @owner.email: sdat@microsoft.com
* @opaqueid CQLD-C28162
* @problem.severity warning
* @precision medium
* @tags correctness
* @scope domainspecific
* @query-version v1
*/

import cpp
import semmle.code.cpp.dataflow.new.DataFlow
import drivers.libraries.SAL
import drivers.kmdf.libraries.KmdfDrivers
import semmle.code.cpp.Specifier

class KernelFloatFunctionAnnotation extends SALAnnotation {
KernelFloatFunctionAnnotation() { this.getMacroName().matches(["_Kernel_float_restored_"]) }
}

class KernelFloatAnnotatedTypedef extends TypedefType {
KernelFloatFunctionAnnotation kernelFloatAnnotation;

KernelFloatAnnotatedTypedef() { kernelFloatAnnotation.getTypedefDeclarations() = this }

KernelFloatFunctionAnnotation getKernelFloatAnnotation() { result = kernelFloatAnnotation }
}

class KernelFloatAnnotatedFunction extends Function {
KernelFloatFunctionAnnotation kernelFloatAnnotation;

cached
KernelFloatAnnotatedFunction() {
(
// this.hasCLinkage() and
exists(
FunctionDeclarationEntry fde // actual function declarations
|
fde = this.getADeclarationEntry() and
kernelFloatAnnotation.getDeclarationEntry() = fde
)
or
exists(
FunctionDeclarationEntry fde // typedefs
|
fde.getFunction() = this and
fde.getTypedefType().(KernelFloatAnnotatedTypedef).getKernelFloatAnnotation() =
kernelFloatAnnotation
)
)
}
}

class SafeFloatRestoreFuncCall extends FunctionCall {
SafeFloatRestoreFuncCall() {
this.getTarget().getName() = ("KeRestoreFloatingPointState") or
this.getTarget().getName() = ("EngRestoreFloatingPointState")
}
}

class SafeFloatRestoreFunc extends Function {
SafeFloatRestoreFunc() {
this.getName() = ("KeRestoreFloatingPointState") or
this.getName() = ("EngRestoreFloatingPointState")
}
}

predicate unused(Expr e) { e instanceof ExprInVoidContext }

from FunctionCall unusedFc, KernelFloatAnnotatedFunction kernelFloatFunc, BasicBlock bb, string msg
where
(
// each path must have a call to a float save function
bb.getEnclosingFunction() = kernelFloatFunc and
not exists(SafeFloatRestoreFuncCall safeFloatRestoreFuncCall, ControlFlowNode node |
bb.getNode(0).getASuccessor*().getBasicBlock().contains(node) and
node = safeFloatRestoreFuncCall.getBasicBlock().getANode() and
safeFloatRestoreFuncCall.getEnclosingFunction() = kernelFloatFunc
) and
msg =
"Function annotated with _Kernel_float_restored_ but does not call a safe float access function for some path(s)"
or
// Paths have call to safe float access function but return value is not used
unusedFc instanceof SafeFloatRestoreFuncCall and
unusedFc.getEnclosingFunction() = kernelFloatFunc and
msg =
"Function annotated with _Kernel_float_restored_ but does not check a safe float access function return value for some path(s)" and
(
// return value isn't used at all
unused(unusedFc)
or
// return value saved to variable but not used
definition(_, unusedFc.getParent()) and
not definitionUsePair(_, unusedFc.getParent(), _)
)
) and
not kernelFloatFunc instanceof SafeFloatRestoreFunc
select kernelFloatFunc, msg
Loading

0 comments on commit 786fda0

Please sign in to comment.