lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CALCETrXy9pKQzx0huPS+Uz8a-sF8dUHynHVQ8NHnGiKtdrUECQ@mail.gmail.com>
Date:	Mon, 11 Jan 2016 16:40:11 -0800
From:	Andy Lutomirski <luto@...capital.net>
To:	"Jeff V. Merkey" <linux.mdb@...il.com>
Cc:	Linus Torvalds <torvalds@...ux-foundation.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>,
	"H. Peter Anvin" <hpa@...or.com>, X86 ML <x86@...nel.org>,
	Peter Zijlstra <peterz@...radead.org>,
	Andy Lutomirski <luto@...nel.org>,
	Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>,
	Steven Rostedt <rostedt@...dmis.org>,
	Borislav Petkov <bp@...en8.de>, Jiri Olsa <jolsa@...nel.org>
Subject: Re: [GIT PULL v4.5] Fix INT1 recursion with unregistered breakpoints

On Mon, Jan 11, 2016 at 4:35 PM, Jeff V. Merkey <linux.mdb@...il.com> wrote:
> The following changes since commit ee78027142ab39d4f3c0e1af71ed343e0ff2dafd:
>
>   Merge pull request #12 from torvalds/master (2015-12-19 11:12:20 -0700)
>
> are available in the git repository at:
>
>
>   https://github.com/jeffmerkey/linux.git fixes
>
> for you to fetch changes up to b5f894bf53e7c401cc5a88b8a8b13059a176a538:
>
>   Fix INT1 Recursion with unregistered breakpoints (2015-12-19 20:33:59 -0700)
>
> ----------------------------------------------------------------
> Jeff Merkey (1):
>       Fix INT1 Recursion with unregistered breakpoints
>
>  arch/x86/include/uapi/asm/debugreg.h |  1 +
>  arch/x86/kernel/hw_breakpoint.c      | 25 +++++++++++++++++++++++--
>  2 files changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/include/uapi/asm/debugreg.h b/arch/x86/include/uapi/asm/debugreg.h
> index 3c0874d..78fc83c 100644
> --- a/arch/x86/include/uapi/asm/debugreg.h
> +++ b/arch/x86/include/uapi/asm/debugreg.h
> @@ -38,6 +38,7 @@
>  #define DR_RW_EXECUTE (0x0)   /* Settings for the access types to trap on */
>  #define DR_RW_WRITE (0x1)
>  #define DR_RW_READ (0x3)
> +#define DR_RW_MASK (0x3) /* mask for breakpoint type field */
>
>  #define DR_LEN_1 (0x0) /* Settings for data length to trap on */
>  #define DR_LEN_2 (0x4)
> diff --git a/arch/x86/kernel/hw_breakpoint.c b/arch/x86/kernel/hw_breakpoint.c
> index 50a3fad..d199834 100644
> --- a/arch/x86/kernel/hw_breakpoint.c
> +++ b/arch/x86/kernel/hw_breakpoint.c
> @@ -444,7 +444,7 @@ EXPORT_SYMBOL_GPL(hw_breakpoint_restore);
>  static int hw_breakpoint_handler(struct die_args *args)
>  {
>         int i, cpu, rc = NOTIFY_STOP;
> -       struct perf_event *bp;
> +       struct perf_event *bp = NULL;
>         unsigned long dr7, dr6;
>         unsigned long *dr6_p;
>
> @@ -477,6 +477,14 @@ static int hw_breakpoint_handler(struct die_args *args)
>                         continue;
>
>                 /*
> +                * Check if we got an execute breakpoint, if so
> +                * set the resume flag to avoid int1 recursion.
> +                */
> +               if (((dr7 >> ((i * DR_CONTROL_SIZE) + DR_CONTROL_SHIFT))
> +                       & DR_RW_MASK) == DR_RW_EXECUTE)
> +                       args->regs->flags |= X86_EFLAGS_RF;
>

I'm still not at all sure I like this, for two reasons:

1. This duplicates the RF-setting logic with all of the legitimate
users.  I can see this blowing up in the case where a breakpoint API
user wants to clear a breakpoint and *not* set RF in case there's
another (possibly just-registered) breakpoint?

2. Shouldn't this clear the spurious breakpoint rather than leaving it set?

In any event, why not just change whatever you're doing to make this
blow up to use the actual breakpoint API?

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ