On 08/12/14 14:47, Kees Cook wrote:
> On Tue, Aug 12, 2014 at 2:39 PM, Stephen Boyd <[email protected]> wrote:
>> On 08/12/14 11:24, Kees Cook wrote:
>>> diff --git a/arch/arm/kernel/patch.c b/arch/arm/kernel/patch.c
>>> index 07314af47733..03dd4e39c833 100644
>>> --- a/arch/arm/kernel/patch.c
>>> +++ b/arch/arm/kernel/patch.c
>>> @@ -13,21 +16,69 @@ struct patch {
>>> unsigned int insn;
>>> };
>>>
>>> -void __kprobes __patch_text(void *addr, unsigned int insn)
>>> +static DEFINE_SPINLOCK(patch_lock);
>>> +
>>> +static void __kprobes *patch_map(void *addr, int fixmap, unsigned long
>>> *flags)
>>> +{
>>> + unsigned int uintaddr = (uintptr_t) addr;
>>> + bool module = !core_kernel_text(uintaddr);
>>> + struct page *page;
>>> +
>>> + if (module && IS_ENABLED(CONFIG_DEBUG_SET_MODULE_RONX))
>>> + page = vmalloc_to_page(addr);
>>> + else if (!module && IS_ENABLED(CONFIG_DEBUG_RODATA))
>>> + page = virt_to_page(addr);
>>> + else
>>> + return addr;
>>> +
>>> + if (flags)
>>> + spin_lock_irqsave(&patch_lock, *flags);
>>> +
>>> + set_fixmap(fixmap, page_to_phys(page));
>>> +
>>> + return (void *) (__fix_to_virt(fixmap) + (uintaddr & ~PAGE_MASK));
>>> +}
>>> +
>>> +static void __kprobes patch_unmap(int fixmap, unsigned long *flags)
>>> +{
>>> + clear_fixmap(fixmap);
>>> +
>>> + if (flags)
>>> + spin_unlock_irqrestore(&patch_lock, *flags);
>>> +}
>> Has the kbuildbot complained about this one yet?
>>
>> CHECK arch/arm/kernel/patch.c
>> arch/arm/kernel/patch.c:47:39: warning: context imbalance in
>> 'patch_unmap' - unexpected unlock
>>
>> I guess we're going to ignore it.
> No, nothing yet from buildbot, let me do a sparse run -- I think we
> can just add annotation and we'll be okay.
>
>
Ok. I tried to move code around but sparse still complains because of
conditional locking.
---8<---
diff --git a/arch/arm/kernel/patch.c b/arch/arm/kernel/patch.c
index 03dd4e39c833..f6d4de3826a0 100644
--- a/arch/arm/kernel/patch.c
+++ b/arch/arm/kernel/patch.c
@@ -18,33 +18,17 @@ struct patch {
static DEFINE_SPINLOCK(patch_lock);
-static void __kprobes *patch_map(void *addr, int fixmap, unsigned long *flags)
+static struct page __kprobes *patch_page(void *addr)
{
unsigned int uintaddr = (uintptr_t) addr;
bool module = !core_kernel_text(uintaddr);
- struct page *page;
if (module && IS_ENABLED(CONFIG_DEBUG_SET_MODULE_RONX))
- page = vmalloc_to_page(addr);
+ return vmalloc_to_page(addr);
else if (!module && IS_ENABLED(CONFIG_DEBUG_RODATA))
- page = virt_to_page(addr);
- else
- return addr;
+ return virt_to_page(addr);
- if (flags)
- spin_lock_irqsave(&patch_lock, *flags);
-
- set_fixmap(fixmap, page_to_phys(page));
-
- return (void *) (__fix_to_virt(fixmap) + (uintaddr & ~PAGE_MASK));
-}
-
-static void __kprobes patch_unmap(int fixmap, unsigned long *flags)
-{
- clear_fixmap(fixmap);
-
- if (flags)
- spin_unlock_irqrestore(&patch_lock, *flags);
+ return NULL;
}
void __kprobes __patch_text_real(void *addr, unsigned int insn, bool remap)
@@ -54,10 +38,18 @@ void __kprobes __patch_text_real(void *addr, unsigned int
insn, bool remap)
bool twopage = false;
unsigned long flags;
void *waddr = addr;
+ struct page *page = NULL;
int size;
- if (remap)
- waddr = patch_map(addr, FIX_TEXT_POKE0, &flags);
+ if (remap) {
+ page = patch_page(addr);
+ if (page) {
+ spin_lock_irqsave(&patch_lock, flags);
+ set_fixmap(FIX_TEXT_POKE0, page_to_phys(page));
+ waddr = (void *)(__fix_to_virt(FIX_TEXT_POKE0) +
+ (uintaddr & ~PAGE_MASK));
+ }
+ }
if (thumb2 && __opcode_is_thumb16(insn)) {
*(u16 *)waddr = __opcode_to_mem_thumb16(insn);
@@ -69,15 +61,25 @@ void __kprobes __patch_text_real(void *addr, unsigned int
insn, bool remap)
u16 *addrh1 = waddr + 2;
twopage = (uintaddr & ~PAGE_MASK) == PAGE_SIZE - 2;
- if (twopage && remap)
- addrh1 = patch_map(addr + 2, FIX_TEXT_POKE1, NULL);
+ if (twopage && remap) {
+ struct page *page2;
+
+ page2 = patch_page(addr + 2);
+ if (page2) {
+ set_fixmap(FIX_TEXT_POKE1, page_to_phys(page));
+ addrh1 = (u16 *)(__fix_to_virt(FIX_TEXT_POKE1) +
+ (uintaddr & ~PAGE_MASK));
+ } else {
+ addrh1 = addr + 2;
+ }
+ }
*addrh0 = __opcode_to_mem_thumb16(first);
*addrh1 = __opcode_to_mem_thumb16(second);
if (twopage && addrh1 != addr + 2) {
flush_kernel_vmap_range(addrh1, 2);
- patch_unmap(FIX_TEXT_POKE1, NULL);
+ clear_fixmap(FIX_TEXT_POKE1);
}
size = sizeof(u32);
@@ -91,9 +93,10 @@ void __kprobes __patch_text_real(void *addr, unsigned int
insn, bool remap)
size = sizeof(u32);
}
- if (waddr != addr) {
+ if (page) {
flush_kernel_vmap_range(waddr, twopage ? size / 2 : size);
- patch_unmap(FIX_TEXT_POKE0, &flags);
+ clear_fixmap(FIX_TEXT_POKE0);
+ spin_unlock_irqrestore(&patch_lock, flags);
}
flush_icache_range((uintptr_t)(addr),
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
