11:43 a.m.
On 21/11/16 18:27, Pino Toscano wrote:
On Wednesday, 9 November 2016 22:38:53 CET Matteo Cafasso wrote:
> The yara_load API allows to load a set of Yara rules contained within a
> file on the host.
>
> Rules can be in binary format, as when compiled with yarac command, or
> in source code format. In the latter case, the rules will be first
> compiled and then loaded.
>
> Subsequent calls of the yara_load API will result in the discard of the
> previously loaded rules.
>
> Signed-off-by: Matteo Cafasso <noxdafox(a)gmail.com>
> ---
> [...]
> diff --git a/daemon/cleanups.c b/daemon/cleanups.c
> index 092e493..a02e521 100644
> --- a/daemon/cleanups.c
> +++ b/daemon/cleanups.c
> @@ -78,3 +93,16 @@ cleanup_free_stringsbuf (void *ptr)
> {
> free_stringsbuf ((struct stringsbuf *) ptr);
> }
> +
> +#ifdef HAVE_YARA
> +
> +void
> +cleanup_destroy_yara_compiler (void *ptr)
> +{
> + YR_COMPILER *compiler = * (YR_COMPILER **) ptr;
> +
> + if (compiler != NULL)
> + yr_compiler_destroy (compiler);
> +}
> +
This should rather be directly in daemon/yara.c, since libyara would be
used there only.
> +static int
> +upload_rules_file (char *rules_path)
> +{
> + int ret = 0;
> + CLEANUP_CLOSE int fd = 0;
> + struct write_callback_data data = { .written = 0 };
> +
> + data.fd = mkstemp (rules_path);
> + if (data.fd == -1) {
> + reply_with_perror ("mkstemp");
> + return -1;
> + }
> +
> + ret = receive_file (write_callback, &data);
> + if (ret == -1) {
> + /* Write error. */
> + cancel_receive ();
> + reply_with_error ("write error: %s", rules_path);
> + return -1;
> + }
> + if (ret == -2) {
> + /* Cancellation from library */
> + reply_with_error ("file upload cancelled");
> + return -1;
> + }
> +
> + return 0;
> +}
This function does not always unlink the temporary file on error, and
it is never close either -- my suggestion would be to reuse and expose
parts of the "upload" function in daemon/upload.c:
int upload_to_fd (int fd)
With the above, upload_rules_file could not be needed anymore, and the
logic to open a temporary fd could be moved directly at the beginning
of do_yara_load.
Just one question: shall the changes in upload.c be in a separate
commit
"expose XYZ"? What is the preferred way?
This question applies also for the notes in PATCH 5.
> +/* Compile source code rules and load them.
> + * Return ERROR_SUCCESS on success, Yara error code type on error.
> + */
> +static int
> +compile_rules_file (const char *rules_path)
> +{
> + int ret = 0;
> + CLEANUP_FCLOSE FILE *rule_file = NULL;
> + CLEANUP_DESTROY_YARA_COMPILER YR_COMPILER *compiler = NULL;
> +
> + ret = yr_compiler_create (&compiler);
> + if (ret != ERROR_SUCCESS) {
> + reply_with_error ("yr_compiler_create");
> + return ret;
> + }
> +
> + yr_compiler_set_callback (compiler, compile_error_callback, NULL);
> +
> + rule_file = fopen (rules_path, "r");
> + if (rule_file == NULL) {
> + reply_with_error ("unable to open rules file");
> + return ret;
> + }
> +
> + ret = yr_compiler_add_file (compiler, rule_file, NULL, rules_path);
Considering it's only a temporary file, and thus we don't show its path
in error message, we could avoid passing rules_path here -- it looks
like the libyara API allows NULL for this parameter.
> + if (ret > 0)
> + return ret;
> +
> + ret = yr_compiler_get_rules (compiler, &rules);
> + if (ret != ERROR_SUCCESS)
> + reply_with_error ("yr_compiler_get_rules");
The API doc say the return value can be either ERROR_SUCCESS or
ERROR_INSUFICENT_MEMORY, so I'd map the latter to ENOMEM and use
reply_with_perror.
> +/* Yara compilation error callback.
> + * Reports back the compilation error message.
> + * Prints compilation warnings if verbose.
> + */
> +static void
> +compile_error_callback(int level, const char *name, int line,
> + const char *message, void *data)
> +{
> + if (level == YARA_ERROR_LEVEL_ERROR)
> + reply_with_error ("(%d): Yara error: %s", line, message);
"Yara error (line %d): %s"
> + else
> + fprintf (stderr, "(%d): Yara warning: %s\n", line, message);
Ditto.
In addition, IMHO this message should be shown only when verbose is
true... like what is written in the API doc comment at the beginning
of the function :)
> +}
> +
> +/* Clean up yara handle on daemon exit. */
> +void yara_finalize (void) __attribute__((destructor));
> +void
> +yara_finalize (void)
> +{
> + int ret = 0;
if (!initialized)
return;
> +
> + if (rules != NULL) {
> + yr_rules_destroy (rules);
> + rules = NULL;
> + }
> +
> + ret = yr_finalize ();
> + if (ret != ERROR_SUCCESS)
> + perror ("yr_finalize");
> +
> + initialized = false;
> +}
Thanks,
_______________________________________________
Libguestfs mailing list
Libguestfs(a)redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs