lkml.org 
[lkml]   [2020]   [Mar]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v3 26/26] objtool: Add STT_NOTYPE noinstr validation
From
Date


On 3/25/20 4:50 PM, Peter Zijlstra wrote:
> On Wed, Mar 25, 2020 at 11:40:46AM -0500, Josh Poimboeuf wrote:
>> On Wed, Mar 25, 2020 at 04:53:48PM +0100, Peter Zijlstra wrote:
>>> On Wed, Mar 25, 2020 at 09:42:11AM -0500, Josh Poimboeuf wrote:
>>>> Sure, but couldn't validate_unwind_hints() and
>>>> validate_reachable_instructions() be changed to *only* run on
>>>> .noinstr.text, for the vmlinux case? That might help converge the
>>>> vmlinux and !vmlinux paths.
>>>
>>> You're thinking something like so then?
>>
>> Not exactly. But I don't want to keep churning this patch set. I can
>> add more patches later, so don't worry about it.
>>
>> But I was thinking it would eventually be good to have the top-level
>> check() be like
>>
>> sec = NULL;
>> if (!validate_all)
>> sec = find_section_by_name(file->elf, ".noinstr.text");
>>
>> ret = validate_functions(&file, sec);
>> if (ret < 0)
>> goto out;
>> warnings += ret;
>>
>> ret = validate_unwind_hints(&file, sec);
>> if (ret < 0)
>> goto out;
>> warnings += ret;
>>
>> if (!warnings) {
>> ret = validate_reachable_instructions(&file, sec);
>> if (ret < 0)
>> goto out;
>> warnings += ret;
>> }
>>
>> if (!validate_all)
>> goto out;
>>
>> ret = validate_retpoline(&file);
>> ....
>>
>> that way the general flow is the same regardless...
>
> Ah,... I see what you mean, there's two little wrinkles with that:
>
> - validate_reachable_instructions() is strictly superluous, it does no
> additional validation between the !vmlinux and vmlinux mode, so I'd
> put that if (!validate_all) goto out, one up.
>
> - tglx has a patch adding .entry.text to be considered as noinstr as a
> whole, which has:
>
>
> @@ -2636,11 +2637,16 @@ static int validate_vmlinux_functions(st
> int warnings = 0;
>
> sec = find_section_by_name(file->elf, ".noinstr.text");
> - if (!sec)
> - return 0;
> + if (sec) {
> + warnings += validate_section(file, sec);
> + warnings += validate_unwind_hints(file, sec);
> + }
>
> - warnings += validate_section(file, sec);
> - warnings += validate_unwind_hints(file, sec);
> + sec = find_section_by_name(file->elf, ".entry.text");
> + if (sec) {
> + warnings += validate_section(file, sec);
> + warnings += validate_unwind_hints(file, sec);
> + }
>
> return warnings;
> }
>
> Anyway, yes, we can do this on top.
>
> I was going to commit the first 17 patches to tip/core/objtool and
> repost the remaining 13 once more. And then see how much pain I did to
> Julien's patches :-)
>

I appreciate the concideration for my patches but, since your patches
have been posted a few times already and were already reviewed, you
might as well commit them. I'll rebase my patches on top and see the
state of things (I'll need to do that with the whole arm64 series anyway).

I'll try to post the new version fast enough so I'm not behind some
other major objtool changes :) .

In the mean time I'll have a look at this series and see what I might
have to change for the rest of the arm64-objtool set!

Thanks,

--
Julien Thierry

\
 
 \ /
  Last update: 2020-03-26 09:02    [W:0.068 / U:2.000 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site