Re: [PATCH 3/3] PR4886: Check build-id in module_init function if able.

[Masami Hiramatsu <mhiramat@redhat.com>]

Hi Wenji,

I'm not sure when the build-id is updated, but interested in your
enhancement. :-)
If I'm using an out-of-tree driver, is build-id updated when I updated
the driver's source code but didn't change running kernel?
In other words, would each driver have its build-id which is different
from other drivers and kernel?

Thank you,

Wenji Huang wrote:
> There are several cases to be considered, whether build-id exists in 
> debuginfo file or not, whether module is loaded or not and whether 
> build-id exists in loaded module/kernel or not.
> ---
>   runtime/sym.c |   66 
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   translate.cxx |    1 +
>   2 files changed, 67 insertions(+), 0 deletions(-)
> 
> diff --git a/runtime/sym.c b/runtime/sym.c
> index 1a9e26b..38323c9 100644
> --- a/runtime/sym.c
> +++ b/runtime/sym.c
> @@ -160,6 +160,72 @@ static const char *_stp_kallsyms_lookup(unsigned 
> long addr, unsigned long *symbo
>          return NULL;
>   }
> 
> +/* Validate module/kernel based on build-id if there
> +*  The completed case is the following combination:
> +*         Debuginfo             Module                          Kernel
> +*                         X                            X
> +*      has build-id/not        unloaded                      has 
> build-id/not
> +*                              loaded && (has build-id/not)
> +*
> +*  NB: build-id exists only if ld>=2.18 and kernel>= 2.6.23
> +*/
> +static int _stp_module_check(void)
> +{
> +       struct _stp_module *m = NULL;
> +       unsigned long notes_addr, base_addr;
> +       unsigned i;
> +
> +       for (i = 0; i < _stp_num_modules; i++)
> +       {
> +               m = _stp_modules[i];
> +
> +               /* unloaded module */
> +               if (m->notes_sect == 0) {
> +                           _stp_warn("skip checking %s\n", m->name);
> +                    continue;
> +                }
> +               if (m->build_id_len > 0) { /* build-id in debuginfo file */
> +                   dbug_sym(1, "validate %s based on build-id\n", m->name);
> +
> +                   /* loaded module/kernel, but without build-id */
> +                   if (m->notes_sect == 1) {
> +                       _stp_error("missing build-id in %s\n", m->name);
> +                       return 1;
> +                   }
> +                   /* notes end address */
> +                   if (!strcmp(m->name, "kernel")) {
> +                         notes_addr = m->build_id_offset;
> +                         base_addr = _stp_module_relocate("kernel",
> +                                                          "_stext", 0);
> +                    } else {
> +                         notes_addr = m->notes_sect + m->build_id_offset;
> +                         base_addr = m->notes_sect;
> +                   }
> +                   /* notes start address */
> +                   notes_addr -= m->build_id_len;
> +                   if (notes_addr > base_addr) {
> +                       if (memcmp(m->build_id_bits,
> +                                   (unsigned char *) notes_addr,
> +                                   m->build_id_len))
> +                       {
> +                          _stp_error("inconsistent build-id in %s with 
> debuginfo\n", m->name);
> +                          return 1;
> +                       }
> +                   } else { /* bug, shouldn't come here */
> +                            _stp_error("unknown failure in checking %s\n",
> +                                                               m->name);
> +                            return 1;
> +                          } /* end comparing */
> +               } else {
> +                         /* build-id in module/kernel, absent in 
> debuginfo */
> +                         if (m->notes_sect > 1) {
> +                           _stp_error("unexpected build-id in %s\n", 
> m->name);
> +                           return 1;
> +                         }
> +                } /* end checking */
> +       } /* end loop */
> +       return 0;
> +}
> 
>   /** Print an address symbolically.
>    * @param address The address to lookup.
> diff --git a/translate.cxx b/translate.cxx
> index a9695f1..0ee5179 100644
> --- a/translate.cxx
> +++ b/translate.cxx
> @@ -1113,6 +1113,7 @@ c_unparser::emit_module_init ()
>     o->newline(-1) << "}";
> 
>     // XXX: perform buildid-based checking if able
> +  o->newline() << "if (_stp_module_check()) rc = -EINVAL;";
> 
>     o->newline(-1) << "}";
>     o->newline() << "if (rc) goto out;";

-- 
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America) Inc.
Software Solutions Division

e-mail: mhiramat@redhat.com