Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 10 Apr 2018 10:23:13 -0400
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: [PATCH] dl_addr: compare addr with sym->st_size.

On Tue, Apr 03, 2018 at 01:06:09PM +0000, Siebenborn, Axel wrote:
> Hi,
> this patch fixes a problem with dl_addr.
> 
> We found symbols, in cases we should not find a symbol, since the
> comparison with sym->st_size is missing.

This was intentional, as my understanding of the historical behavior
on other implementations was that it would do this. If that's
incorrect we should investigate and document (or find existing
documentation of) what they really do.

> According to the spec, dl_addr should not return an error in this
> case. Instead dli_sname and dli_addr should be set to NULL.

OK, I found that part in the man page.

> diff --git a/ldso/dynlink.c b/ldso/dynlink.c
> index 9bf6924..cc87dc0 100644
> --- a/ldso/dynlink.c
> +++ b/ldso/dynlink.c
> @@ -1958,7 +1958,7 @@ int dladdr(const void *addr, Dl_info *info)
>                  && (1<<(sym->st_info&0xf) & OK_TYPES)
>                  && (1<<(sym->st_info>>4) & OK_BINDS)) {
>                         void *symaddr = laddr(p, sym->st_value);
> -                       if (symaddr > addr || symaddr < best)
> +                       if (symaddr > addr || (void*) ((uint8_t*) symaddr + sym->st_size) < addr || symaddr < best)

Not all symbols have st_size set. In that case the old "best match"
behavior should probably be kept unless there's a strong reason not
to.

>                                 continue;
>                         best = symaddr;
>                         bestsym = sym;
> @@ -1967,13 +1967,16 @@ int dladdr(const void *addr, Dl_info *info)
>                 }
>         }
>  
> -       if (!best) return 0;
> -
> -       if (DL_FDPIC && (bestsym->st_info&0xf) == STT_FUNC)
> -               best = p->funcdescs + (bestsym - p->syms);
> -
>         info->dli_fname = p->name;
>         info->dli_fbase = p->map;
> +       if (!best) {
> +               info->dli_sname = 0;
> +               info->dli_saddr = 0;
> +               return 0

This is missing a ; so it seems you tested a slightly different patch..?

> +       }
> +
> +       if ( DL_FDPIC && (bestsym->st_info&0xf) == STT_FUNC)
> +               best = p->funcdescs + (bestsym - p->syms);
>         info->dli_sname = strings + bestsym->st_name;
>         info->dli_saddr = best;

Otherwise this looks ok but I haven't tested it.

Rich

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.