Re: Backtrace library [3/3]

[Ian Lance Taylor <iant at google dot com>]

On Wed, Sep 12, 2012 at 2:01 AM, Florian Weimer <fweimer@redhat.com> wrote:
>
> +backtrace_open (const char *filename, backtrace_error_callback
> error_callback,
> +               void *data)
> +{
> +  int descriptor;
> +
> +  descriptor = open (filename, O_RDONLY | O_CLOEXEC);
> +  if (descriptor < 0)
> +    {
> +      error_callback (data, filename, errno);
> +      return -1;
> +    }
> +  if (O_CLOEXEC == 0)
> +    {
> +      /* It doesn't matter if this fails for some reason.  */
> +      fcntl (descriptor, F_SETFD, FD_CLOEXEC);
> +    }
>
> You should call fcntl unconditionally.  O_CLOEXEC might be non-zero during
> build, but could still be ignored by the kernel.

OK, done.

> +static void
> +fileline_initialize (backtrace_error_callback error_callback, void *data)
> +{
> ...
> +  if (executable_name != NULL)
> +    descriptor = backtrace_open (executable_name, error_callback, data);
> +  else
> +    descriptor = backtrace_open ("/proc/self/exe", error_callback, data);
>
> You should try getauxval(AT_EXECFN) as well (needs recent glibc), so that
> this works with a mounted /proc.

I'm going to postpone this--my glibc doesn't have this function.

> This library should only be used when getauxval(AT_SECURE) is zero, so that
> the program doesn't try to read files with elevated privileges to which the
> original user wouldn't have access.  I don't think this has to be addressed
> within the library itself.

The library doesn't try to elevate privileges, so, yes, I think this
is entirely the responsibility of the program calling the library.

> Adding /usr/lib/debug support shouldn't be too hard, I will try to figure
> out the required path transformations (which are somewhat system-specific).

Thanks!

Ian