linux-kernel - Re: [REVIEW][PATCH 02/20] signal/x86: Inline fill_sigtrap_info in it's only caller send

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: <87pnxaf0hl.fsf@xmission.com>
Date:   Wed, 19 Sep 2018 08:46:30 +0200
From:   ebiederm@...ssion.com (Eric W. Biederman)
To:     Christoph Hellwig <hch@...radead.org>
Cc:     linux-kernel@...r.kernel.org, linux-arch@...r.kernel.org,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, x86@...nel.org
Subject: Re: [REVIEW][PATCH 02/20] signal/x86: Inline fill_sigtrap_info in it's only caller send_sigtrap

Christoph Hellwig <hch@...radead.org> writes:

>>  
>>  	clear_siginfo(&info);
>> -	fill_sigtrap_info(tsk, regs, error_code, si_code, &info);
>> +	tsk->thread.trap_nr = X86_TRAP_DB;
>> +	tsk->thread.error_code = error_code;
>> +
>> +	info.si_signo = SIGTRAP;
>> +	info.si_code = si_code;
>> +	info.si_addr = user_mode(regs) ? (void __user *)regs->ip : NULL;
>
> clear_siginfo already zeroes the whole structure, so this could be
> written more clearly as:
>
> 	if (user_mode(regs)
> 		info.si_addr = (void __user *)regs->ip;

That change does not make sense in this particular patch as it is just
code motion.

It also does not make sense in the final version of the code at
the end of the patch series which is:

void send_sigtrap(struct task_struct *tsk, struct pt_regs *regs,
					 int error_code, int si_code)
{
	tsk->thread.trap_nr = X86_TRAP_DB;
	tsk->thread.error_code = error_code;

	/* Send us the fake SIGTRAP */
	force_sig_fault(SIGTRAP, si_code,
			user_mode(regs) ? (void __user *)regs->ip : NULL, tsk);
}

In this version the test also makes sense because struct siginfo is
gone because manually filling it out results in more bugs than
necessary.  That is now left up to force_sig_fault.

I was hoping that we could show that user_mode(regs) is always true.
But according to arch/x86/kernel/traps.c:do_debug watch points that will
trigger even when the kernel accesses user space addresses.

Eric