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

Add PANDA_PLUGIN_PATH #1408

Open
wants to merge 11 commits into
base: dev
Choose a base branch
from
27 changes: 12 additions & 15 deletions panda/python/core/pandare/panda.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,11 @@ def __init__(self, arch="i386", mem="128M",
self.catch_exceptions=catch_exceptions
self.qlog = QEMU_Log_Manager(self)
self.build_dir = None
self.plugin_path = plugin_path
prev_path = environ.get("PANDA_PLUGIN_PATH")
if plugin_path and prev_path:
environ["PANDA_PLUGIN_PATH"] = f"{plugin_path}:{prev_path}"
elif plugin_path:
environ["PANDA_PLUGIN_PATH"] = plugin_path

self.serial_unconsumed_data = b''

Expand Down Expand Up @@ -259,19 +263,12 @@ def __init__(self, arch="i386", mem="128M",
progress ("Panda args: [" + (" ".join(self.panda_args)) + "]")
# /__init__

def get_plugin_path(self):
if self.plugin_path is None:
build_dir = self.get_build_dir()
rel_dir = pjoin(*[build_dir, self.arch_name+"-softmmu", "panda", "plugins"])

if build_dir == "/usr/local/bin/":
# Installed - use /usr/local/lib/panda/plugins
self.plugin_path = f"/usr/local/lib/panda/{self.arch_name}"
elif isdir(rel_dir):
self.plugin_path = rel_dir
else:
raise ValueError(f"Could not find plugin path. Build dir={build_dir}")
return self.plugin_path
def get_plugin_path(self, plugin_name):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this doesn't seem to cover all the cases from before, was the /usr/local/bin special-casing redundant? it also doesn't appear to perform error propagation/checking, so this is a breaking change (as catching ValueError to handle missing plugins isn't possible)

plugin_name_ffi = self.ffi.new("char[]", bytes(plugin_name, "utf-8"))
plugin_path_ffi = self.libpanda.panda_plugin_path(plugin_name_ffi)
plugin_path = self.ffi.string(plugin_path_ffi).decode("utf-8")
# TODO: self.libpanda.g_free(plugin_path_ffi)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

leftover TODO comment

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure this is a memory leak without the free, but CFFI doesn't seem to find the g_free() function, which is weird because other parts of the code find g_malloc0(). I don't know what's going on here.

return plugin_path

def get_build_dir(self):
if self.build_dir is None:
Expand Down Expand Up @@ -946,7 +943,7 @@ def _load_plugin_library(self, name):
libpanda_path_chr = self.ffi.new("char[]",bytes(self.libpanda_path, "UTF-8"))
self.__did_load_libpanda = self.libpanda.panda_load_libpanda(libpanda_path_chr)
if not name in self.plugins.keys():
plugin = pjoin(*[self.get_plugin_path(), f"panda_{name}.so"])
plugin = self.get_plugin_path(name)
assert(isfile(plugin))
self.plugins[name] = self.ffi.dlopen(plugin)

Expand Down
17 changes: 16 additions & 1 deletion panda/src/callbacks.c
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,7 @@ extern const char *qemu_file;
// - Relative to the QEMU binary
// - Relative to the standard install location (/usr/local/lib/panda/[arch]/)
// - Relative to the install prefix directory.
// - In the directories in the PANDA_PLUGIN_PATH environment variable.
char* resolve_file_from_plugin_directory(const char* file_name_fmt, const char* name){
char *plugin_path, *name_formatted;
// makes "taint2" -> "panda_taint2"
Expand Down Expand Up @@ -357,7 +358,7 @@ char* resolve_file_from_plugin_directory(const char* file_name_fmt, const char*
return plugin_path;
}

// Finally, try relative to the installation path.
// Fourth, try relative to the installation path.
plugin_path = attempt_normalize_path(
g_strdup_printf("%s/%s/%s", CONFIG_PANDA_PLUGINDIR,
TARGET_NAME, name_formatted));
Expand All @@ -366,6 +367,20 @@ char* resolve_file_from_plugin_directory(const char* file_name_fmt, const char*
}
g_free(plugin_path);

// Finally, try the directories in PANDA_PLUGIN_PATH
gchar **panda_plugin_path = g_strsplit(g_getenv("PANDA_PLUGIN_PATH"), ":", -1);
for (gchar **dir = panda_plugin_path; *dir; dir++) {
gchar *plugin_path = attempt_normalize_path(g_strdup_printf(
"%s/%s/%s", *dir,
TARGET_NAME, name_formatted));
if (TRUE == g_file_test(plugin_path, G_FILE_TEST_EXISTS)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor nit but don't think you need to actually check against glibc TRUE here. just checking for non-zero return is a superset of checking for glib TRUE, so there's no real reason to write in this style.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can change it but all the other tests in the function use the same style so it would feel inconsistent.

g_strfreev(panda_plugin_path);
return plugin_path;
}
g_free(plugin_path);
}
g_strfreev(panda_plugin_path);

// Return null if plugin resolution failed.
return NULL;
}
Expand Down
Loading