Messages in this thread | | | Subject | Re: [PATCH V4 2/9] jump_label: Add the jump_label_can_update_check() helper | From | Daniel Bristot de Oliveira <> | Date | Thu, 7 Feb 2019 18:00:53 +0100 |
| |
On 2/7/19 3:08 PM, Borislav Petkov wrote: > On Thu, Feb 07, 2019 at 02:21:09PM +0100, Daniel Bristot de Oliveira wrote: >> diff --git a/kernel/jump_label.c b/kernel/jump_label.c >> index 288d630da22d..1e6f4d27e28d 100644 >> --- a/kernel/jump_label.c >> +++ b/kernel/jump_label.c >> @@ -374,22 +374,29 @@ static enum jump_label_type jump_label_type(struct jump_entry *entry) >> return enabled ^ branch; >> } >> >> +static bool jump_label_can_update(struct jump_entry *entry, bool init) >> +{ >> + /* >> + * Cannot update code that was in an init text area. >> + */ >> + if (!init || jump_entry_is_init(entry)) > > Shouldn't this be && > > ?
Oops! should be &&! sorry.
>> + return false; >> + >> + if (WARN_ONCE(!kernel_text_address(jump_entry_code(entry)), >> + "can't patch jump_label at %pS", (void *)jump_entry_code(entry))) >> + return false; > > Yeah, I think that this way of writing it is less readable than: > > if (!kernel_text_address(jump_entry_code(entry))) { > WARN_ONCE(1, "can't patch jump_label at %pS", (void *)jump_entry_code(entry)); > return false; > }
It is taking 95 characters. In this case, wouldn't be better to break?
if (!kernel_text_address(jump_entry_code(entry))) { WARN_ONCE(1, "can't patch jump_label at %pS", (void *)jump_entry_code(entry)); return false; }
I agree your suggestion is better... just confirming that 95 is not too long...
> >> + if (jump_label_can_update(entry, init)) { >> + arch_jump_label_transform(entry, jump_label_type(entry)); > > Yap. > > Thx. >
Thanks!
-- Daniel
| |