Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch to CodeQL to detect prohibited function use #15819

Merged
merged 1 commit into from
Jan 26, 2024

Conversation

ryao
Copy link
Contributor

@ryao ryao commented Jan 25, 2024

Motivation and Context

The LLVM/Clang developers pointed out that using the CPP to detect use of functions that our QA policies prohibit risks invoking undefined behavior.

Description

We remove the CPP macros from automake in favor of adding a custom CodeQL query.

How Has This Been Tested?

I have tested this in my fork of the openzfs/zfs github repository with an experimental patch that added strncpy() to the codebase:

diff --git a/module/zfs/arc.c b/module/zfs/arc.c
index 3bcffb3c7ede..75a104251336 100644
--- a/module/zfs/arc.c
+++ b/module/zfs/arc.c
@@ -4432,6 +4432,11 @@ arc_kmem_reap_soon(void)
 	kmem_cache_t		*prev_data_cache = NULL;
 
 #ifdef _KERNEL
+	char source[] = "example";
+	char destination[10];
+
+	strncpy(destination, source, sizeof(destination));
+	destination[sizeof(destination) - 1] = '\0';
 #if defined(_ILP32)
 	/*
 	 * Reclaim unused memory from all kmem caches.

The issue was detected and the appropriate message was shown in the security tab. I did not test a PR to see what would be displayed, but I assume that the message would also show there, just as it does for the stock CodeQL queries. I can push a test commit to this PR to verify that if people feel it is necessary.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist:

The LLVM/Clang developers pointed out that using the CPP to detect use
of functions that our QA policies prohibit risks invoking undefined
behavior. To resolve this, we configure CodeQL to detect forbidden
function usage.

Note that cpp in the context of CodeQL refers to C/C++, rather than the
C PreProcessor, which C++ also uses. It really should have been written
cxx, but that ship sailed a long time ago. This misuse of the term cpp
is retained in the CodeQL configuration for consistency with upstream
CodeQL.

As a side benefit, verbose make no longer is a wall of text showing a
bunch of CPP macros, which can make debugging slightly easier.

Signed-off-by: Richard Yao <[email protected]>
Closes openzfs#14134
@behlendorf behlendorf added the Status: Accepted Ready to integrate (reviewed, tested) label Jan 26, 2024
@behlendorf behlendorf merged commit e7af89d into openzfs:master Jan 26, 2024
22 of 25 checks passed
behlendorf pushed a commit to behlendorf/zfs that referenced this pull request Jan 29, 2024
The LLVM/Clang developers pointed out that using the CPP to detect use
of functions that our QA policies prohibit risks invoking undefined
behavior. To resolve this, we configure CodeQL to detect forbidden
function usage.

Note that cpp in the context of CodeQL refers to C/C++, rather than the
C PreProcessor, which C++ also uses. It really should have been written
cxx, but that ship sailed a long time ago. This misuse of the term cpp
is retained in the CodeQL configuration for consistency with upstream
CodeQL.

As a side benefit, verbose make no longer is a wall of text showing a
bunch of CPP macros, which can make debugging slightly easier.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Richard Yao <[email protected]>
Closes openzfs#15819 
Closes openzfs#14134
behlendorf pushed a commit that referenced this pull request Jan 29, 2024
The LLVM/Clang developers pointed out that using the CPP to detect use
of functions that our QA policies prohibit risks invoking undefined
behavior. To resolve this, we configure CodeQL to detect forbidden
function usage.

Note that cpp in the context of CodeQL refers to C/C++, rather than the
C PreProcessor, which C++ also uses. It really should have been written
cxx, but that ship sailed a long time ago. This misuse of the term cpp
is retained in the CodeQL configuration for consistency with upstream
CodeQL.

As a side benefit, verbose make no longer is a wall of text showing a
bunch of CPP macros, which can make debugging slightly easier.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Richard Yao <[email protected]>
Closes #15819 
Closes #14134
lundman pushed a commit to openzfsonwindows/openzfs that referenced this pull request Mar 13, 2024
The LLVM/Clang developers pointed out that using the CPP to detect use
of functions that our QA policies prohibit risks invoking undefined
behavior. To resolve this, we configure CodeQL to detect forbidden
function usage.

Note that cpp in the context of CodeQL refers to C/C++, rather than the
C PreProcessor, which C++ also uses. It really should have been written
cxx, but that ship sailed a long time ago. This misuse of the term cpp
is retained in the CodeQL configuration for consistency with upstream
CodeQL.

As a side benefit, verbose make no longer is a wall of text showing a
bunch of CPP macros, which can make debugging slightly easier.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Richard Yao <[email protected]>
Closes openzfs#15819 
Closes openzfs#14134
lundman pushed a commit to openzfsonwindows/openzfs that referenced this pull request Mar 13, 2024
The LLVM/Clang developers pointed out that using the CPP to detect use
of functions that our QA policies prohibit risks invoking undefined
behavior. To resolve this, we configure CodeQL to detect forbidden
function usage.

Note that cpp in the context of CodeQL refers to C/C++, rather than the
C PreProcessor, which C++ also uses. It really should have been written
cxx, but that ship sailed a long time ago. This misuse of the term cpp
is retained in the CodeQL configuration for consistency with upstream
CodeQL.

As a side benefit, verbose make no longer is a wall of text showing a
bunch of CPP macros, which can make debugging slightly easier.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Richard Yao <[email protected]>
Closes openzfs#15819 
Closes openzfs#14134
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants