linux-kernel - Re: [PATCH] x86/ptrace: Add comments to x86

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] [day] [month] [year] [list]

Message-ID: <20180715225433.GA30102@gmail.com>
Date:   Mon, 16 Jul 2018 00:54:33 +0200
From:   Ingo Molnar <mingo@...nel.org>
To:     Yu-cheng Yu <yu-cheng.yu@...el.com>
Cc:     linux-kernel@...r.kernel.org, x86@...nel.org,
        "H. Peter Anvin" <hpa@...or.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>
Subject: Re: [PATCH] x86/ptrace: Add comments to x86_regset about empty slots


* Yu-cheng Yu <yu-cheng.yu@...el.com> wrote:

> find_regset() goes through regsets sequentially.  Empty slots
> in regset arrays causes mismatch.  Add comments to x86_regset
> enum.
> 
> Signed-off-by: Yu-cheng Yu <yu-cheng.yu@...el.com>
> ---
>  arch/x86/kernel/ptrace.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/arch/x86/kernel/ptrace.c b/arch/x86/kernel/ptrace.c
> index e2ee403865eb..130ca4f27a17 100644
> --- a/arch/x86/kernel/ptrace.c
> +++ b/arch/x86/kernel/ptrace.c
> @@ -42,6 +42,11 @@
>  
>  #include "tls.h"
>  
> +/*
> + * find_regset() goes through regsets sequentially.
> + * Do not create empty slots in x86_64_regsets[] or
> + * x86_32_regsets[] below.
> + */
>  enum x86_regset {
>  	REGSET_GENERAL,
>  	REGSET_FP,

What mismatch exactly? The logic in find_regset() is:

        for (n = 0; n < view->n; ++n) {
                regset = view->regsets + n;
                if (regset->core_note_type == type)
                        return regset;
        }


and an 'empty' slot would have .core_note_type of 0 - which would be easy to skip 
or warn about.

It appears to me user-space ptrace users can control 'type' via PTRACE_GETREGSET, 
so it might make sense to filter out the value of 0 there.

Or something like that. The patch doesn't really explain the problem, and I 
maintain my argument that the current code of relying on no empty slots and not 
having any mechanism other than human review ensuring it is both ugly and fragile.

Thanks,

	Ingo