Skip to content

Commit

Permalink
Add and updateCodeQL suppression
Browse files Browse the repository at this point in the history 
Microsoft CodeQL analyzer's suppression format is slightly different
than GitHub's, and expects the suppression comment to be one line.
Update suppression comments in `pkg\ociwclayer\import.go` to conform.

Suppress warnings for "uncontrolled process operation" in `init\init.c`
and `vsockexec\vsockexec.c`.
Suppress "incorrect conversion between integer types" in
`internal\jobobject\limits.go`, and add fix to
`internal\guest\runtime\hcsv2\uvm.go`.

Signed-off-by: Hamza El-Saawy <[email protected]>
  • Loading branch information
@helsaawy
helsaawy committed Aug 23, 2024
1 parent e7a1be7 commit 740e632
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 49 deletions.
87 changes: 44 additions & 43 deletions init/init.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ const char *const default_envp[] = {
};

#ifdef MODULES
// global kmod k_ctx so we can access it in the file tree traversal
// global kmod k_ctx so we can access it in the file tree traversal
struct kmod_ctx *k_ctx;
#endif

Expand Down Expand Up @@ -357,7 +357,7 @@ void init_entropy(int port) {
close(e);
}

// dmesg is a helper function for printing to dmesg. We cannot assume that the
// dmesg is a helper function for printing to dmesg. We cannot assume that the
// image has syslogd or similar running for now, so we cannot just use syslog(3).
//
// /dev/kmsg exports the structured data in the following line format:
Expand All @@ -366,7 +366,7 @@ void dmesg(const unsigned int level, const char *msg)
{
int fd_kmsg = open("/dev/kmsg", O_WRONLY);
if (fd_kmsg == -1) {
// failed to open the kmsg device
// failed to open the kmsg device
warn("error opening /dev/kmsg");
return;
}
Expand All @@ -384,7 +384,7 @@ void dmesg(const unsigned int level, const char *msg)
}
}

// see https://man7.org/linux/man-pages/man2/syslog.2.html for definitions of levels
// see https://man7.org/linux/man-pages/man2/syslog.2.html for definitions of levels
void dmesgErr(const char *msg) { dmesg(3, msg); }
void dmesgWarn(const char *msg) { dmesg(4, msg); }
void dmesgInfo(const char *msg) { dmesg(6, msg); }
Expand Down Expand Up @@ -415,7 +415,8 @@ pid_t launch(int argc, char **argv) {
if (putenv(DEFAULT_PATH_ENV)) { // Specify the PATH used for execvpe
die("putenv");
}
execvpe(argvn[0], argvn, (char**)default_envp);
// CodeQL [SM01925] designed to initialize Linux guest and then exec the command-line arguments (either ./vsockexec or ./cmd/gcs)
execvpe(argvn[0], argvn, (char **)default_envp);
die2("execvpe", argvn[0]);
}

Expand Down Expand Up @@ -444,10 +445,10 @@ int reap_until(pid_t until_pid) {
}

#ifdef MODULES
// load_module gets the module from the absolute path to the module and then
// inserts into the kernel.
// load_module gets the module from the absolute path to the module and then
// inserts into the kernel.
int load_module(struct kmod_ctx *ctx, const char *module_path) {
struct kmod_module *mod = NULL;
struct kmod_module *mod = NULL;
int err;

#ifdef DEBUG
Expand All @@ -456,71 +457,71 @@ int load_module(struct kmod_ctx *ctx, const char *module_path) {

err = kmod_module_new_from_path(ctx, module_path, &mod);
if (err < 0) {
return err;
return err;
}

err = kmod_module_probe_insert_module(mod, 0, NULL, NULL, NULL, NULL);
err = kmod_module_probe_insert_module(mod, 0, NULL, NULL, NULL, NULL);
if (err < 0) {
kmod_module_unref(mod);
kmod_module_unref(mod);
return err;
}

kmod_module_unref(mod);
kmod_module_unref(mod);
return 0;
}

// parse_tree_entry is called by ftw for each directory and file in the file tree.
// If this entry is a file and has a .ko file extension, attempt to load into kernel.
int parse_tree_entry(const char *fpath, const struct stat *sb, int typeflag) {
int result;
const char *ext;
const char *ext;

if (typeflag != FTW_F) {
// do nothing if this isn't a file
return 0;
// do nothing if this isn't a file
return 0;
}

ext = strrchr(fpath, '.');
if (!ext || ext == fpath) {
// no file extension found in the filepath
return 0;
return 0;
}

if ((result = strcmp(ext, ".ko")) != 0) {
// file does not have .ko extension so it is not a kernel module
return 0;
return 0;
}

// print warning if we fail to load the module, but don't fail fn so
// we keep trying to load the rest of the modules.
// print warning if we fail to load the module, but don't fail fn so
// we keep trying to load the rest of the modules.
result = load_module(k_ctx, fpath);
if (result != 0) {
warn2("failed to load module", fpath);
}
return 0;
}

// load_all_modules finds the modules in the image and loads them using kmod,
// which accounts for ordering requirements.
// load_all_modules finds the modules in the image and loads them using kmod,
// which accounts for ordering requirements.
void load_all_modules() {
int max_path = 256;
char modules_dir[max_path];
struct utsname uname_data;
int ret;
int ret;

// get information on the running kernel
// get information on the running kernel
ret = uname(&uname_data);
if (ret != 0) {
die("failed to get kernel information");
}
// create the absolute path of the modules directory this looks

// create the absolute path of the modules directory this looks
// like /lib/modules/<uname.release>
ret = snprintf(modules_dir, max_path, "%s/%s", lib_modules, uname_data.release);
ret = snprintf(modules_dir, max_path, "%s/%s", lib_modules, uname_data.release);
if (ret < 0) {
die("failed to create the modules directory path");
} else if (ret > max_path) {
die("modules directory buffer larger than expected");
die("modules directory buffer larger than expected");
}

if (k_ctx == NULL) {
Expand All @@ -531,14 +532,14 @@ void load_all_modules() {
}

kmod_load_resources(k_ctx);
ret = ftw(modules_dir, parse_tree_entry, OPEN_FDS);
ret = ftw(modules_dir, parse_tree_entry, OPEN_FDS);
if (ret != 0) {
// Don't fail on error from walking the file tree and loading modules right now.
// Don't fail on error from walking the file tree and loading modules right now.
// ftw may return an error if the modules directory doesn't exist, which
// may be the case for some images. Additionally, we don't currently support
// using a denylist when loading modules, so we may try to load modules
// that cannot be loaded until later, such as nvidia modules which fail to
// load if no device is present.
// using a denylist when loading modules, so we may try to load modules
// that cannot be loaded until later, such as nvidia modules which fail to
// load if no device is present.
warn("error adding modules");
}

Expand Down Expand Up @@ -595,39 +596,39 @@ int debug_main(int argc, char **argv) {
}
#endif

// start_services is a helper function to start different services that are
// expected to be running in the guest on boot. These processes run as
// linux daemons.
// start_services is a helper function to start different services that are
// expected to be running in the guest on boot. These processes run as
// linux daemons.
//
// Future work: Support collecting logs for these services and handle
// Future work: Support collecting logs for these services and handle
// log rotation as needed.
void start_services() {
// While execvpe will already search the path for the executable, it does
// so after forking. We can avoid that unnecessary fork by stating the
// binary beforehand.
// so after forking. We can avoid that unnecessary fork by stating the
// binary beforehand.
char *persistenced_name = "/bin/nvidia-persistenced";
struct stat persistenced_stat;
struct stat persistenced_stat;
if (stat(persistenced_name, &persistenced_stat) == -1) {
dmesgWarn("nvidia-persistenced not present, skipping ");
} else {
dmesgInfo("start nvidia-persistenced daemon");
pid_t persistenced_pid = launch(1, &persistenced_name);
if (persistenced_pid < 0) {
// do not return early if we fail to start this, since it's possible that
// do not return early if we fail to start this, since it's possible that
// this service doesn't exist on the system, which is a valid scenario
dmesgWarn("failed to start nvidia-persistenced daemon");
}
}

char *fm_name = "/bin/nv-fabricmanager";
struct stat fabric_stat;
struct stat fabric_stat;
if (stat(fm_name, &fabric_stat) == -1) {
dmesgWarn("nv-fabricmanager not present, skipping ");
} else {
dmesgInfo("start nv-fabricmanager daemon");
pid_t fm_pid = launch(1, &fm_name);
if (fm_pid < 0) {
// do not return early if we fail to start this, since it's possible that
// do not return early if we fail to start this, since it's possible that
// this service doesn't exist on the system, which is a valid scenario
dmesgWarn("failed to start nv-fabricmanager daemon");
}
Expand Down
3 changes: 3 additions & 0 deletions internal/guest/runtime/hcsv2/uvm.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -824,6 +824,9 @@ func (h *Host) GetProperties(ctx context.Context, containerID string, query prot
}
properties.ProcessList = make([]prot.ProcessDetails, len(pids))
for i, pid := range pids {
if outOfUint32Bounds(pid) {
return nil, errors.Errorf("PID (%d) exceeds uint32 bounds", pid)
}
properties.ProcessList[i].ProcessID = uint32(pid)
}
} else if requestedProperty == prot.PtStatistics {
Expand Down
1 change: 1 addition & 0 deletions internal/jobobject/limits.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ func (job *JobObject) SetCPUAffinity(affinityBitMask uint64) error {
return fmt.Errorf("affinity bitmask (%d) exceeds max allowable value (%d)", affinityBitMask, maxUintptr)
}

// CodeQL [SM03681] checked against max value above (there is no math.MaxUintPtr ...)
info.BasicLimitInformation.Affinity = uintptr(affinityBitMask)
return job.setExtendedInformation(info)
}
Expand Down
9 changes: 3 additions & 6 deletions pkg/ociwclayer/import.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,7 @@ func ImportLayerFromTar(ctx context.Context, r io.Reader, path string, parentLay

func writeLayerFromTar(ctx context.Context, r io.Reader, w wclayer.LayerWriter, root string) (int64, error) {
t := tar.NewReader(r)
// CodeQL [SM03409] False positive, `internal/safefile` package ensures tar extractions are always
// bound to the layer root directory.
// CodeQL [SM03409] `internal\wclayer` uses `internal/safefile` to bind tar extraction to the layer's root directory
hdr, err := t.Next()
totalSize := int64(0)
buf := bufio.NewWriter(nil)
Expand All @@ -80,16 +79,14 @@ func writeLayerFromTar(ctx context.Context, r io.Reader, w wclayer.LayerWriter,
if err != nil {
return 0, err
}
// CodeQL [SM03409] False positive, `internal/safefile` package ensures tar extractions are always
// bound to the layer root directory.
// CodeQL [SM03409] `internal\wclayer` uses `internal/safefile` to bind tar extraction to the layer's root directory
hdr, err = t.Next()
} else if hdr.Typeflag == tar.TypeLink {
err = w.AddLink(filepath.FromSlash(hdr.Name), filepath.FromSlash(hdr.Linkname))
if err != nil {
return 0, err
}
// CodeQL [SM03409] False positive, `internal/safefile` package ensures tar extractions are always
// bound to the layer root directory.
// CodeQL [SM03409] `internal\wclayer` uses `internal/safefile` to bind tar extraction to the layer's root directory
hdr, err = t.Next()
} else {
var (
Expand Down
1 change: 1 addition & 0 deletions vsockexec/vsockexec.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ int main(int argc, char **argv)
}
}

// CodeQL [SM01925] designed to forward stdio over VSOCK and then exec the command-line arguments (always ./cmd/gcs)
execvp(argv[optind], argv + optind);
fprintf(stderr, "execvp: %s: %s\n", argv[optind], strerror(errno));
return 1;
Expand Down

0 comments on commit 740e632

Please sign in to comment.