[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Libunwind-devel] [PATCH][x86_64] Make address validation a per thre
From: |
David Mosberger-Tang |
Subject: |
Re: [Libunwind-devel] [PATCH][x86_64] Make address validation a per thread setting |
Date: |
Mon, 16 Jun 2008 14:36:48 -0600 |
Applied, thanks.
--david
On 6/9/08, Arun Sharma <address@hidden> wrote:
> For local unwinding, we have a defence mechanism against bad/missing
> unwind information, which could result in libunwind dereferencing
> bad pointers. This mechanism is based on msync(2) system call and
> significantly reduces the chances of a bad pointer dereference in
> libunwind.
>
> The original idea was to turn this mechanism on only when necessary
> i.e. libunwind didn't find proper unwind information for a IP.
>
> There are a couple of problems in the current implementation.
>
> * The flag is global and is modified without locking
> * The flag isn't reset when starting a new unwind
>
> The attached patch makes ->validate a per-thread setting by moving
> it into struct cursor from unw_local_addr_space and resets it to
> false when starting a new unwind. As a result, cursor->as_arg points
> to the cursor itself instead of the ucontext (for the local case).
>
> This was found to reduce the number of msync() system calls from an
> application using libunwind significantly.
>
> Signed-off-by: Paul Pluzhnikov <address@hidden>
> Signed-off-by: Arun Sharma <address@hidden>
>
> --- a/include/tdep-x86_64/libunwind_i.h
> +++ b/include/tdep-x86_64/libunwind_i.h
> @@ -51,7 +51,6 @@ struct unw_addr_space
> unw_word_t dyn_info_list_addr; /* (cached) dyn_info_list_addr */
> struct dwarf_rs_cache global_cache;
> struct unw_debug_frame_list *debug_frames;
> - int validate;
> };
>
> struct cursor
> @@ -67,8 +66,17 @@ struct cursor
> }
> sigcontext_format;
> unw_word_t sigcontext_addr;
> + int validate;
> + ucontext_t *uc;
> };
>
> +static inline ucontext_t *
> +dwarf_get_uc(const struct dwarf_cursor *cursor)
> +{
> + const struct cursor *c = (struct cursor *) cursor->as_arg;
> + return c->uc;
> +}
> +
> #define DWARF_GET_LOC(l) ((l).val)
>
> #ifdef UNW_LOCAL_ONLY
> @@ -77,10 +85,10 @@ struct cursor
> # define DWARF_LOC(r, t) ((dwarf_loc_t) { .val = (r) })
> # define DWARF_IS_REG_LOC(l) 0
> # define DWARF_REG_LOC(c,r) (DWARF_LOC((unw_word_t)
> \
> - tdep_uc_addr((c)->as_arg, (r)), 0))
> + tdep_uc_addr(dwarf_get_uc(c), (r)), 0))
> # define DWARF_MEM_LOC(c,m) DWARF_LOC ((m), 0)
> # define DWARF_FPREG_LOC(c,r) (DWARF_LOC((unw_word_t)
> \
> - tdep_uc_addr((c)->as_arg, (r)), 0))
> + tdep_uc_addr(dwarf_get_uc(c), (r)), 0))
> #else /* !UNW_LOCAL_ONLY */
>
> # define DWARF_LOC_TYPE_FP (1 << 0)
> diff --git a/src/x86_64/Ginit.c b/src/x86_64/Ginit.c
> index 026e8d2..031deaa 100644
> --- a/src/x86_64/Ginit.c
> +++ b/src/x86_64/Ginit.c
> @@ -158,7 +158,8 @@ access_mem (unw_addr_space_t as, unw_word_t addr,
> unw_word_t *val, int write,
> else
> {
> /* validate address */
> - if (as->validate && validate_mem(addr))
> + const struct cursor *c = (const struct cursor *)arg;
> + if (c && c->validate && validate_mem(addr))
> return -1;
> *val = *(unw_word_t *) addr;
> Debug (16, "mem[%016lx] -> %lx\n", addr, *val);
> @@ -171,7 +172,7 @@ access_reg (unw_addr_space_t as, unw_regnum_t reg,
> unw_word_t *val, int write,
> void *arg)
> {
> unw_word_t *addr;
> - ucontext_t *uc = arg;
> + ucontext_t *uc = ((struct cursor *)arg)->uc;
>
> if (unw_is_fpreg (reg))
> goto badreg;
> @@ -200,7 +201,7 @@ static int
> access_fpreg (unw_addr_space_t as, unw_regnum_t reg, unw_fpreg_t *val,
> int write, void *arg)
> {
> - ucontext_t *uc = arg;
> + ucontext_t *uc = ((struct cursor *)arg)->uc;
> unw_fpreg_t *addr;
>
> if (!unw_is_fpreg (reg))
> @@ -252,7 +253,6 @@ x86_64_local_addr_space_init (void)
> local_addr_space.acc.get_proc_name = get_static_proc_name;
> unw_flush_cache (&local_addr_space, 0, 0);
>
> - local_addr_space.validate = 0;
> memset (last_good_addr, 0, sizeof (unw_word_t) * NLGA);
> lga_victim = 0;
> }
> diff --git a/src/x86_64/Ginit_local.c b/src/x86_64/Ginit_local.c
> index 9bb2513..1e80cb8 100644
> --- a/src/x86_64/Ginit_local.c
> +++ b/src/x86_64/Ginit_local.c
> @@ -49,7 +49,9 @@ unw_init_local (unw_cursor_t *cursor, ucontext_t *uc)
> Debug (1, "(cursor=%p)\n", c);
>
> c->dwarf.as = unw_local_addr_space;
> - c->dwarf.as_arg = uc;
> + c->dwarf.as_arg = c;
> + c->uc = uc;
> + c->validate = 0;
> return common_init (c);
> }
>
> diff --git a/src/x86_64/Ginit_remote.c b/src/x86_64/Ginit_remote.c
> index 9b2fd23..8aa644d 100644
> --- a/src/x86_64/Ginit_remote.c
> +++ b/src/x86_64/Ginit_remote.c
> @@ -42,7 +42,16 @@ unw_init_remote (unw_cursor_t *cursor, unw_addr_space_t
> as, void *as_arg)
> Debug (1, "(cursor=%p)\n", c);
>
> c->dwarf.as = as;
> - c->dwarf.as_arg = as_arg;
> + if (as == unw_local_addr_space)
> + {
> + c->dwarf.as_arg = c;
> + c->uc = as_arg;
> + }
> + else
> + {
> + c->dwarf.as_arg = as_arg;
> + c->uc = 0;
> + }
> return common_init (c);
> #endif /* !UNW_LOCAL_ONLY */
> }
> diff --git a/src/x86_64/Gis_signal_frame.c b/src/x86_64/Gis_signal_frame.c
> index b795827..72edf46 100644
> --- a/src/x86_64/Gis_signal_frame.c
> +++ b/src/x86_64/Gis_signal_frame.c
> @@ -40,7 +40,6 @@ unw_is_signal_frame (unw_cursor_t *cursor)
>
> as = c->dwarf.as;
> a = unw_get_accessors (as);
> - as->validate = 1; /* Don't trust the ip */
> arg = c->dwarf.as_arg;
>
> /* Check if RIP points at sigreturn sequence.
> diff --git a/src/x86_64/Gresume.c b/src/x86_64/Gresume.c
> index 4edc4da..b400040 100644
> --- a/src/x86_64/Gresume.c
> +++ b/src/x86_64/Gresume.c
> @@ -51,7 +51,7 @@ x86_64_local_resume (unw_addr_space_t as, unw_cursor_t
> *cursor, void *arg)
> {
> #if defined(__linux)
> struct cursor *c = (struct cursor *) cursor;
> - ucontext_t *uc = c->dwarf.as_arg;
> + ucontext_t *uc = c->uc;
>
> /* Ensure c->pi is up-to-date. On x86-64, it's relatively common to
> be missing DWARF unwind info. We don't want to fail in that
> diff --git a/src/x86_64/Gstep.c b/src/x86_64/Gstep.c
> index 75f796f..2da1c25 100644
> --- a/src/x86_64/Gstep.c
> +++ b/src/x86_64/Gstep.c
> @@ -71,6 +71,10 @@ unw_step (unw_cursor_t *cursor)
> unw_word_t prev_ip = c->dwarf.ip, prev_cfa = c->dwarf.cfa;
> struct dwarf_loc rbp_loc, rsp_loc, rip_loc;
>
> + /* We could get here because of missing/bad unwind information.
> + Validate all addresses before dereferencing. */
> + c->validate = 1;
> +
> Debug (13, "dwarf_step() failed (ret=%d), trying frame-chain\n", ret);
>
> if (unw_is_signal_frame (cursor))
>
>
> _______________________________________________
> Libunwind-devel mailing list
> address@hidden
> http://lists.nongnu.org/mailman/listinfo/libunwind-devel
>
--
Mosberger Consulting LLC, http://www.mosberger-consulting.com/