linux-kernel - Re: [PATCH 1/3] jump_label: Warn on failed jump

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: <a4ad6f69-8836-779b-f7e7-0e303885c7f5@akamai.com>
Date:   Wed, 14 Feb 2018 12:18:20 -0500
From:   Jason Baron <jbaron@...mai.com>
To:     Steven Rostedt <rostedt@...dmis.org>,
        Josh Poimboeuf <jpoimboe@...hat.com>
Cc:     x86@...nel.org, linux-kernel@...r.kernel.org,
        Ingo Molnar <mingo@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Borislav Petkov <bp@...e.de>
Subject: Re: [PATCH 1/3] jump_label: Warn on failed jump_label patch



On 02/14/2018 12:01 PM, Steven Rostedt wrote:
> On Wed, 14 Feb 2018 10:40:41 -0600
> Josh Poimboeuf <jpoimboe@...hat.com> wrote:
> 
>> When the jump label code encounters an address which isn't recognized by
>> kernel_text_address(), it just silently fails.
>>
>> This can be dangerous because jump labels are used in a variety of
>> places, and are generally expected to work.  Convert the silent failure
>> to a warning.
> 
> I made ftrace function tracing work on init code, can we do the same
> with tracepoints (aka jump labels)?
> 

jump labels do work on init code, except they don't work on it after it
has been freed :)

It uses 'kernel_text_address()', which will return true for init code if
system_state < SYSTEM_RUNNING. See: core_kernel_text().

So I'm guessing that the warnings here are coming from init code that
has already been freed. Are we sure that these warnings are coming from
init code that hasn't already been freed?

Thanks,

-Jason

> But I have to say that this goes with my argument that there exists
> tracepoints in the kernel that nobody cares about ;-)
> 
> -- Steve
> 
>>
>> Signed-off-by: Josh Poimboeuf <jpoimboe@...hat.com>
>> ---
>>  kernel/jump_label.c | 8 ++++++--
>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/kernel/jump_label.c b/kernel/jump_label.c
>> index b4517095db6a..c71fb7cdfc41 100644
>> --- a/kernel/jump_label.c
>> +++ b/kernel/jump_label.c
>> @@ -370,8 +370,12 @@ static void __jump_label_update(struct static_key *key,
>>  		 * kernel_text_address() verifies we are not in core kernel
>>  		 * init code, see jump_label_invalidate_module_init().
>>  		 */
>> -		if (entry->code && kernel_text_address(entry->code))
>> -			arch_jump_label_transform(entry, jump_label_type(entry));
>> +		if (entry->code) {
>> +			if (kernel_text_address(entry->code))
>> +				arch_jump_label_transform(entry, jump_label_type(entry));
>> +			else
>> +				WARN_ONCE(1, "can't patch jump_label at %pS", (void *)entry->code);
>> +		}
>>  	}
>>  }
>>  
>