-
Notifications
You must be signed in to change notification settings - Fork 28
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge pull request #138 from microsoft/development
RI of development branch to main (03/29/24)
- Loading branch information
Showing
36 changed files
with
2,345 additions
and
83 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
55 changes: 55 additions & 0 deletions
55
src/drivers/general/queries/MultithreadedAVCondition/MultithreadedAVCondition.qhelp
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| <!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd"> | ||
| <qhelp> | ||
| <overview> | ||
| <p> | ||
| This warning indicates that a thread has potential to access deleted objects if preempted. Unlike the Code Analysis version of this query, this query does not currently verify the use of any synchronization mechanisms, so it may produce false positives. | ||
| </p> | ||
| </overview> | ||
| <recommendation> | ||
| <p> | ||
| There should be no access to a reference-counted object after the reference count is at zero | ||
| </p> | ||
| </recommendation> | ||
| <example> | ||
| <p> | ||
| In this example, m_cRef is a member of this. A thread T1 executes the if condition, decrements m_cRef to 1, and is then preempted. Another thread T2 executes the if condition, decrements m_cRef to 0, executes the if body (where this is deleted), and returns NULL. | ||
| </p> | ||
| <sample language="c"><![CDATA[ | ||
| ULONG Release_bad() | ||
| { | ||
| if (0 == InterlockedDecrement(&m_cRef)) | ||
| { | ||
| delete this; | ||
| return NULL; | ||
| } | ||
| /* this.m_cRef isn't thread safe */ | ||
| return m_cRef; | ||
| } | ||
| ]]> | ||
| </sample> | ||
| <p> | ||
| The following code does not reference any heap memory after the object is deleted. | ||
| </p> | ||
| <sample language="c"><![CDATA[ | ||
| ULONG CObject::Release() | ||
| { | ||
| ASSERT(0 != m_cRef); | ||
| ULONG cRef = InterlockedDecrement(&m_cRef); | ||
| if (0 == cRef) | ||
| { | ||
| delete this; | ||
| return NULL; | ||
| } | ||
| return cRef; | ||
| } | ||
| ]]> | ||
| </sample> | ||
| </example> | ||
| <references> | ||
| <li> | ||
| <a href="https://learn.microsoft.com/en-us/windows-hardware/drivers/devtest/28616-multithreaded-av-condition"> | ||
| Warning C28616 | ||
| </a> | ||
| </li> | ||
| </references> | ||
| </qhelp> |
37 changes: 37 additions & 0 deletions
37
src/drivers/general/queries/MultithreadedAVCondition/MultithreadedAVCondition.ql
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| // Copyright (c) Microsoft Corporation. | ||
| // Licensed under the MIT license. | ||
| /** | ||
| * @id cpp/drivers/multithreaded-av-condition | ||
| * @name Multithreaded Access Violation Condition | ||
| * @description This warning indicates that a thread has potential to access deleted objects if preempted. | ||
| * @platform Desktop | ||
| * @security.severity Medium | ||
| * @feature.area Multiple | ||
| * @impact Exploitable Design | ||
| * @repro.text There should be no access to a reference-counted object after the reference count is at zero | ||
| * @owner.email sdat@microsoft.com | ||
| * @opaqueid CQLD-C28616 | ||
| * @kind problem | ||
| * @problem.severity warning | ||
| * @precision medium | ||
| * @tags correctness | ||
| * wddst | ||
| * @scope domainspecific | ||
| * @query-version v1 | ||
| */ | ||
|
|
||
| import cpp | ||
| import semmle.code.cpp.ir.IR | ||
|
|
||
| from BasicBlock delBlock, BasicBlock useBlock, ThisExpr t, PointerFieldAccess p | ||
| where | ||
| exists(DeleteExpr del | del.getExpr() = t) and | ||
| t.getEnclosingDeclaration() = p.getQualifier().getEnclosingDeclaration() and | ||
| p.getEnclosingDeclaration() = t.getEnclosingDeclaration() and | ||
| delBlock = t.getBasicBlock() and | ||
| useBlock = p.getBasicBlock() and | ||
| not useBlock.contains(delBlock) and | ||
| not delBlock.contains(useBlock) and | ||
| not delBlock.getAPredecessor*() = useBlock and | ||
| delBlock.getAPredecessor*() = useBlock.getAPredecessor*() | ||
| select p, "Possible Multithreaded Access Violation. Object deleted $@ but member $@ referenced", t, "here", p, p.toString() |
Oops, something went wrong.