Skip to content

Commit e836673

Browse files
anadavIngo Molnar
authored andcommitted
x86/alternatives: Add text_poke_kgdb() to not assert the lock when debugging
text_mutex is currently expected to be held before text_poke() is called, but kgdb does not take the mutex, and instead *supposedly* ensures the lock is not taken and will not be acquired by any other core while text_poke() is running. The reason for the "supposedly" comment is that it is not entirely clear that this would be the case if gdb_do_roundup is zero. Create two wrapper functions, text_poke() and text_poke_kgdb(), which do or do not run the lockdep assertion respectively. While we are at it, change the return code of text_poke() to something meaningful. One day, callers might actually respect it and the existing BUG_ON() when patching fails could be removed. For kgdb, the return value can actually be used. Suggested-by: Peter Zijlstra <peterz@infradead.org> Signed-off-by: Nadav Amit <namit@vmware.com> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org> Acked-by: Jiri Kosina <jkosina@suse.cz> Cc: <akpm@linux-foundation.org> Cc: <ard.biesheuvel@linaro.org> Cc: <deneen.t.dock@intel.com> Cc: <kernel-hardening@lists.openwall.com> Cc: <kristen@linux.intel.com> Cc: <linux_dti@icloud.com> Cc: <will.deacon@arm.com> Cc: Andy Lutomirski <luto@kernel.org> Cc: Borislav Petkov <bp@alien8.de> Cc: Dave Hansen <dave.hansen@intel.com> Cc: H. Peter Anvin <hpa@zytor.com> Cc: Kees Cook <keescook@chromium.org> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Rik van Riel <riel@surriel.com> Cc: Thomas Gleixner <tglx@linutronix.de> Fixes: 9222f60 ("x86/alternatives: Lockdep-enforce text_mutex in text_poke*()") Link: https://lkml.kernel.org/r/20190426001143.4983-2-namit@vmware.com Signed-off-by: Ingo Molnar <mingo@kernel.org>
1 parent d5963d8 commit e836673

File tree

3 files changed

+45
-19
lines changed

3 files changed

+45
-19
lines changed

arch/x86/include/asm/text-patching.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ extern void *text_poke_early(void *addr, const void *opcode, size_t len);
3535
* inconsistent instruction while you patch.
3636
*/
3737
extern void *text_poke(void *addr, const void *opcode, size_t len);
38+
extern void *text_poke_kgdb(void *addr, const void *opcode, size_t len);
3839
extern int poke_int3_handler(struct pt_regs *regs);
3940
extern void *text_poke_bp(void *addr, const void *opcode, size_t len, void *handler);
4041
extern int after_bootmem;

arch/x86/kernel/alternative.c

Lines changed: 38 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -679,18 +679,7 @@ void *__init_or_module text_poke_early(void *addr, const void *opcode,
679679
return addr;
680680
}
681681

682-
/**
683-
* text_poke - Update instructions on a live kernel
684-
* @addr: address to modify
685-
* @opcode: source of the copy
686-
* @len: length to copy
687-
*
688-
* Only atomic text poke/set should be allowed when not doing early patching.
689-
* It means the size must be writable atomically and the address must be aligned
690-
* in a way that permits an atomic write. It also makes sure we fit on a single
691-
* page.
692-
*/
693-
void *text_poke(void *addr, const void *opcode, size_t len)
682+
static void *__text_poke(void *addr, const void *opcode, size_t len)
694683
{
695684
unsigned long flags;
696685
char *vaddr;
@@ -703,8 +692,6 @@ void *text_poke(void *addr, const void *opcode, size_t len)
703692
*/
704693
BUG_ON(!after_bootmem);
705694

706-
lockdep_assert_held(&text_mutex);
707-
708695
if (!core_kernel_text((unsigned long)addr)) {
709696
pages[0] = vmalloc_to_page(addr);
710697
pages[1] = vmalloc_to_page(addr + PAGE_SIZE);
@@ -733,6 +720,43 @@ void *text_poke(void *addr, const void *opcode, size_t len)
733720
return addr;
734721
}
735722

723+
/**
724+
* text_poke - Update instructions on a live kernel
725+
* @addr: address to modify
726+
* @opcode: source of the copy
727+
* @len: length to copy
728+
*
729+
* Only atomic text poke/set should be allowed when not doing early patching.
730+
* It means the size must be writable atomically and the address must be aligned
731+
* in a way that permits an atomic write. It also makes sure we fit on a single
732+
* page.
733+
*/
734+
void *text_poke(void *addr, const void *opcode, size_t len)
735+
{
736+
lockdep_assert_held(&text_mutex);
737+
738+
return __text_poke(addr, opcode, len);
739+
}
740+
741+
/**
742+
* text_poke_kgdb - Update instructions on a live kernel by kgdb
743+
* @addr: address to modify
744+
* @opcode: source of the copy
745+
* @len: length to copy
746+
*
747+
* Only atomic text poke/set should be allowed when not doing early patching.
748+
* It means the size must be writable atomically and the address must be aligned
749+
* in a way that permits an atomic write. It also makes sure we fit on a single
750+
* page.
751+
*
752+
* Context: should only be used by kgdb, which ensures no other core is running,
753+
* despite the fact it does not hold the text_mutex.
754+
*/
755+
void *text_poke_kgdb(void *addr, const void *opcode, size_t len)
756+
{
757+
return __text_poke(addr, opcode, len);
758+
}
759+
736760
static void do_sync_core(void *info)
737761
{
738762
sync_core();

arch/x86/kernel/kgdb.c

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -759,13 +759,13 @@ int kgdb_arch_set_breakpoint(struct kgdb_bkpt *bpt)
759759
if (!err)
760760
return err;
761761
/*
762-
* It is safe to call text_poke() because normal kernel execution
762+
* It is safe to call text_poke_kgdb() because normal kernel execution
763763
* is stopped on all cores, so long as the text_mutex is not locked.
764764
*/
765765
if (mutex_is_locked(&text_mutex))
766766
return -EBUSY;
767-
text_poke((void *)bpt->bpt_addr, arch_kgdb_ops.gdb_bpt_instr,
768-
BREAK_INSTR_SIZE);
767+
text_poke_kgdb((void *)bpt->bpt_addr, arch_kgdb_ops.gdb_bpt_instr,
768+
BREAK_INSTR_SIZE);
769769
err = probe_kernel_read(opc, (char *)bpt->bpt_addr, BREAK_INSTR_SIZE);
770770
if (err)
771771
return err;
@@ -784,12 +784,13 @@ int kgdb_arch_remove_breakpoint(struct kgdb_bkpt *bpt)
784784
if (bpt->type != BP_POKE_BREAKPOINT)
785785
goto knl_write;
786786
/*
787-
* It is safe to call text_poke() because normal kernel execution
787+
* It is safe to call text_poke_kgdb() because normal kernel execution
788788
* is stopped on all cores, so long as the text_mutex is not locked.
789789
*/
790790
if (mutex_is_locked(&text_mutex))
791791
goto knl_write;
792-
text_poke((void *)bpt->bpt_addr, bpt->saved_instr, BREAK_INSTR_SIZE);
792+
text_poke_kgdb((void *)bpt->bpt_addr, bpt->saved_instr,
793+
BREAK_INSTR_SIZE);
793794
err = probe_kernel_read(opc, (char *)bpt->bpt_addr, BREAK_INSTR_SIZE);
794795
if (err || memcmp(opc, bpt->saved_instr, BREAK_INSTR_SIZE))
795796
goto knl_write;

0 commit comments

Comments
 (0)