[Libguestfs] [PATCH 3/3] daemon/yara: fix undefined behavior due to Yara 4.0 API changes

[Eric Blake]

On Tue, Oct 12, 2021 at 12:36:27AM +0200, Laszlo Ersek wrote:
> The prototype of yara_rules_callback() is:
> 
> > static int
> > yara_rules_callback (int code, void *message, void *data)
> 
> however, in Yara commit 2b121b166d25 ("Track string matches using
> YR_SCAN_CONTEXT.", 2020-02-27), which was included in the upstream v4.0.0
> release, the rules callback prototype was changed as follows:
> 
> > diff --git a/libyara/include/yara/types.h b/libyara/include/yara/types.h
> > index cad095cd70c2..f415033c4aa6 100644
> > --- a/libyara/include/yara/types.h
> > +++ b/libyara/include/yara/types.h
> > @@ -661,6 +644,7 @@ struct YR_MEMORY_BLOCK_ITERATOR
> >
> >
> >  typedef int (*YR_CALLBACK_FUNC)(
> > +    YR_SCAN_CONTEXT* context,
> >      int message,
> >      void* message_data,
> >      void* user_data);

Do we intend to compile against both older and newer versions of Yara,
in which case we'd need a configure-time probe of which variant we
must compile against?  I could not quickly find documentation of a
minimum version of Yara that we are willing to support, at least not
in README or HACKING.

> Of course, evaluating this expression would be undefined behavior, but in
> the GCC extension expression
> 
>   typeof (*(YR_CALLBACK_FUNC)NULL)
> 
> the operand of the "typeof" operator is never evaluated, as it does not
> have a variably modified type. We can therefore use this "typeof" in the
> same role as HELLO_FUNC had in the above example.

Clever.

> +/* Typedefs that effectively strip the pointer derivation from Yara's
> + * YR_CALLBACK_FUNC and YR_COMPILER_CALLBACK_FUNC types, using GCC's "typeof"
> + * extension.
> + */
> +typedef typeof (*(YR_CALLBACK_FUNC)NULL)          guestfs_yr_callback;
> +typedef typeof (*(YR_COMPILER_CALLBACK_FUNC)NULL) guestfs_yr_compiler_callback;
> +
> +/* Declarations of our callback functions expressed in terms of Yara's
> + * typedefs. Note: these are *function declarations*.
> + */
> +static guestfs_yr_callback          yara_rules_callback;
> +static guestfs_yr_compiler_callback compile_error_callback;
> +

As written, this portion of the code will compile with either old or
new Yara...

>  /* Has one FileIn parameter.
>   * Takes optional arguments, consult optargs_bitmask.
>   */
> @@ -210,7 +221,7 @@ compile_rules_file (const char *rules_path)
>   */
>  static void
>  compile_error_callback (int level, const char *name, int line,
> -                        const char *message, void *data)
> +                        const YR_RULE* rule, const char *message, void *data)

...but this will cause compile failure on old Yara (redefinition to an
incompatible type); if we want to support both versions, we need
#ifdef alternatives depending on the version of Yara we are building
against.

[I did not check whether the Yara project properly bumped their .so
version to match their incompatible ABI change; hopefully they did, so
that anyone using dynamic linking against older libraries will be
forced to recompile for the new .so, rather than being hit with
runtime failures as you encountered in the libguestfs testsuite.  But
if not, that's a bug report for a different mailing list]

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org