<html> <head> <meta content="text/html; charset=windows-1252" http-equiv="Content-Type"> </head> <body bgcolor="#FFFFFF" text="#000000"> On 21/11/16 18:27, Pino Toscano wrote: <blockquote cite="mid:2515384.8rruaeMXFo@thyrus.usersys.redhat.com" type="cite"> <pre wrap="">On Wednesday, 9 November 2016 22:38:53 CET Matteo Cafasso wrote: </pre> <blockquote type="cite"> <pre wrap="">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 <a class="moz-txt-link-rfc2396E" href="mailto:noxdafox@gmail.com"><noxdafox@gmail.com></a> --- [...] 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); +} + </pre> </blockquote> <pre wrap=""> This should rather be directly in daemon/yara.c, since libyara would be used there only. </pre> <blockquote type="cite"> <pre wrap="">+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; +} </pre> </blockquote> <pre wrap=""> 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.</pre> </blockquote> 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. <blockquote cite="mid:2515384.8rruaeMXFo@thyrus.usersys.redhat.com" type="cite"> <pre wrap=""> </pre> <blockquote type="cite"> <pre wrap="">+/* 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); </pre> </blockquote> <pre wrap=""> 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. </pre> <blockquote type="cite"> <pre wrap="">+ if (ret > 0) + return ret; + + ret = yr_compiler_get_rules (compiler, &rules); + if (ret != ERROR_SUCCESS) + reply_with_error ("yr_compiler_get_rules"); </pre> </blockquote> <pre wrap=""> 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. </pre> <blockquote type="cite"> <pre wrap="">+/* 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); </pre> </blockquote> <pre wrap=""> "Yara error (line %d): %s" </pre> <blockquote type="cite"> <pre wrap="">+ else + fprintf (stderr, "(%d): Yara warning: %s\n", line, message); </pre> </blockquote> <pre wrap=""> 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 :) </pre> <blockquote type="cite"> <pre wrap="">+} + +/* Clean up yara handle on daemon exit. */ +void yara_finalize (void) __attribute__((destructor)); +void +yara_finalize (void) +{ + int ret = 0; </pre> </blockquote> <pre wrap=""> if (!initialized) return; </pre> <blockquote type="cite"> <pre wrap="">+ + if (rules != NULL) { + yr_rules_destroy (rules); + rules = NULL; + } + + ret = yr_finalize (); + if (ret != ERROR_SUCCESS) + perror ("yr_finalize"); + + initialized = false; +} </pre> </blockquote> <pre wrap=""> Thanks, </pre> <fieldset class="mimeAttachmentHeader"></fieldset> <pre wrap="">_______________________________________________ Libguestfs mailing list <a class="moz-txt-link-abbreviated" href="mailto:Libguestfs@redhat.com">Libguestfs@redhat.com</a> <a class="moz-txt-link-freetext" href="https://www.redhat.com/mailman/listinfo/libguestfs">https://www.redhat.com/mailman/listinfo/libguestfs</a></pre> </blockquote> </body> </html>