Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

kdb: Get rid of custom debug heap allocator

Currently the only user for debug heap is kdbnearsym() which can be
modified to rather use statically allocated buffer for symbol name as
per it's current usage. So do that and hence remove custom debug heap
allocator.

Note that this change puts a restriction on kdbnearsym() callers to
carefully use shared namebuf such that a caller should consume the symbol
returned immediately prior to another call to fetch a different symbol.

Also, this change uses standard KSYM_NAME_LEN macro for namebuf
allocation instead of local variable: knt1_size which should avoid any
conflicts caused by changes to KSYM_NAME_LEN macro value.

This change has been tested using kgdbtest on arm64 which doesn't show
any regressions.

Suggested-by: Daniel Thompson <daniel.thompson@linaro.org>
Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
Link: https://lore.kernel.org/r/20210714055620.369915-1-sumit.garg@linaro.org
Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>

authored by

Sumit Garg and committed by
Daniel Thompson
95f7f154 2734d6c1

+28 -307
-1
kernel/debug/kdb/kdb_debugger.c
··· 140 140 */ 141 141 kdb_common_deinit_state(); 142 142 KDB_STATE_CLEAR(PAGER); 143 - kdbnearsym_cleanup(); 144 143 if (error == KDB_CMD_KGDB) { 145 144 if (KDB_STATE(DOING_KGDB)) 146 145 KDB_STATE_CLEAR(DOING_KGDB);
-5
kernel/debug/kdb/kdb_private.h
··· 109 109 long *, char **); 110 110 extern int kdbgetsymval(const char *, kdb_symtab_t *); 111 111 extern int kdbnearsym(unsigned long, kdb_symtab_t *); 112 - extern void kdbnearsym_cleanup(void); 113 112 extern char *kdb_strdup(const char *str, gfp_t type); 114 113 extern void kdb_symbol_print(unsigned long, const kdb_symtab_t *, unsigned int); 115 114 ··· 231 232 #define kdb_task_has_cpu(p) (task_curr(p)) 232 233 233 234 #define GFP_KDB (in_dbg_master() ? GFP_ATOMIC : GFP_KERNEL) 234 - 235 - extern void *debug_kmalloc(size_t size, gfp_t flags); 236 - extern void debug_kfree(void *); 237 - extern void debug_kusage(void); 238 235 239 236 extern struct task_struct *kdb_current_task; 240 237 extern struct pt_regs *kdb_current_regs;
+28 -301
kernel/debug/kdb/kdb_support.c
··· 52 52 } 53 53 EXPORT_SYMBOL(kdbgetsymval); 54 54 55 - static char *kdb_name_table[100]; /* arbitrary size */ 56 - 57 - /* 58 - * kdbnearsym - Return the name of the symbol with the nearest address 59 - * less than 'addr'. 55 + /** 56 + * kdbnearsym() - Return the name of the symbol with the nearest address 57 + * less than @addr. 58 + * @addr: Address to check for near symbol 59 + * @symtab: Structure to receive results 60 60 * 61 - * Parameters: 62 - * addr Address to check for symbol near 63 - * symtab Structure to receive results 64 - * Returns: 65 - * 0 No sections contain this address, symtab zero filled 66 - * 1 Address mapped to module/symbol/section, data in symtab 67 - * Remarks: 68 - * 2.6 kallsyms has a "feature" where it unpacks the name into a 69 - * string. If that string is reused before the caller expects it 70 - * then the caller sees its string change without warning. To 71 - * avoid cluttering up the main kdb code with lots of kdb_strdup, 72 - * tests and kfree calls, kdbnearsym maintains an LRU list of the 73 - * last few unique strings. The list is sized large enough to 74 - * hold active strings, no kdb caller of kdbnearsym makes more 75 - * than ~20 later calls before using a saved value. 61 + * WARNING: This function may return a pointer to a single statically 62 + * allocated buffer (namebuf). kdb's unusual calling context (single 63 + * threaded, all other CPUs halted) provides us sufficient locking for 64 + * this to be safe. The only constraint imposed by the static buffer is 65 + * that the caller must consume any previous reply prior to another call 66 + * to lookup a new symbol. 67 + * 68 + * Note that, strictly speaking, some architectures may re-enter the kdb 69 + * trap if the system turns out to be very badly damaged and this breaks 70 + * the single-threaded assumption above. In these circumstances successful 71 + * continuation and exit from the inner trap is unlikely to work and any 72 + * user attempting this receives a prominent warning before being allowed 73 + * to progress. In these circumstances we remain memory safe because 74 + * namebuf[KSYM_NAME_LEN-1] will never change from '\0' although we do 75 + * tolerate the possibility of garbled symbol display from the outer kdb 76 + * trap. 77 + * 78 + * Return: 79 + * * 0 - No sections contain this address, symtab zero filled 80 + * * 1 - Address mapped to module/symbol/section, data in symtab 76 81 */ 77 82 int kdbnearsym(unsigned long addr, kdb_symtab_t *symtab) 78 83 { 79 84 int ret = 0; 80 85 unsigned long symbolsize = 0; 81 86 unsigned long offset = 0; 82 - #define knt1_size 128 /* must be >= kallsyms table size */ 83 - char *knt1 = NULL; 87 + static char namebuf[KSYM_NAME_LEN]; 84 88 85 89 kdb_dbg_printf(AR, "addr=0x%lx, symtab=%px\n", addr, symtab); 86 90 memset(symtab, 0, sizeof(*symtab)); 87 91 88 92 if (addr < 4096) 89 93 goto out; 90 - knt1 = debug_kmalloc(knt1_size, GFP_ATOMIC); 91 - if (!knt1) { 92 - kdb_func_printf("addr=0x%lx cannot kmalloc knt1\n", addr); 93 - goto out; 94 - } 94 + 95 95 symtab->sym_name = kallsyms_lookup(addr, &symbolsize , &offset, 96 - (char **)(&symtab->mod_name), knt1); 96 + (char **)(&symtab->mod_name), namebuf); 97 97 if (offset > 8*1024*1024) { 98 98 symtab->sym_name = NULL; 99 99 addr = offset = symbolsize = 0; ··· 102 102 symtab->sym_end = symtab->sym_start + symbolsize; 103 103 ret = symtab->sym_name != NULL && *(symtab->sym_name) != '\0'; 104 104 105 - if (ret) { 106 - int i; 107 - /* Another 2.6 kallsyms "feature". Sometimes the sym_name is 108 - * set but the buffer passed into kallsyms_lookup is not used, 109 - * so it contains garbage. The caller has to work out which 110 - * buffer needs to be saved. 111 - * 112 - * What was Rusty smoking when he wrote that code? 113 - */ 114 - if (symtab->sym_name != knt1) { 115 - strncpy(knt1, symtab->sym_name, knt1_size); 116 - knt1[knt1_size-1] = '\0'; 117 - } 118 - for (i = 0; i < ARRAY_SIZE(kdb_name_table); ++i) { 119 - if (kdb_name_table[i] && 120 - strcmp(kdb_name_table[i], knt1) == 0) 121 - break; 122 - } 123 - if (i >= ARRAY_SIZE(kdb_name_table)) { 124 - debug_kfree(kdb_name_table[0]); 125 - memmove(kdb_name_table, kdb_name_table+1, 126 - sizeof(kdb_name_table[0]) * 127 - (ARRAY_SIZE(kdb_name_table)-1)); 128 - } else { 129 - debug_kfree(knt1); 130 - knt1 = kdb_name_table[i]; 131 - memmove(kdb_name_table+i, kdb_name_table+i+1, 132 - sizeof(kdb_name_table[0]) * 133 - (ARRAY_SIZE(kdb_name_table)-i-1)); 134 - } 135 - i = ARRAY_SIZE(kdb_name_table) - 1; 136 - kdb_name_table[i] = knt1; 137 - symtab->sym_name = kdb_name_table[i]; 138 - knt1 = NULL; 139 - } 140 - 141 105 if (symtab->mod_name == NULL) 142 106 symtab->mod_name = "kernel"; 143 107 kdb_dbg_printf(AR, "returns %d symtab->sym_start=0x%lx, symtab->mod_name=%px, symtab->sym_name=%px (%s)\n", 144 108 ret, symtab->sym_start, symtab->mod_name, symtab->sym_name, symtab->sym_name); 145 - 146 109 out: 147 - debug_kfree(knt1); 148 110 return ret; 149 - } 150 - 151 - void kdbnearsym_cleanup(void) 152 - { 153 - int i; 154 - for (i = 0; i < ARRAY_SIZE(kdb_name_table); ++i) { 155 - if (kdb_name_table[i]) { 156 - debug_kfree(kdb_name_table[i]); 157 - kdb_name_table[i] = NULL; 158 - } 159 - } 160 111 } 161 112 162 113 static char ks_namebuf[KSYM_NAME_LEN+1], ks_namebuf_prev[KSYM_NAME_LEN+1]; ··· 605 654 { 606 655 char state[] = { kdb_task_state_char(p), '\0' }; 607 656 return (mask & kdb_task_state_string(state)) != 0; 608 - } 609 - 610 - /* Last ditch allocator for debugging, so we can still debug even when 611 - * the GFP_ATOMIC pool has been exhausted. The algorithms are tuned 612 - * for space usage, not for speed. One smallish memory pool, the free 613 - * chain is always in ascending address order to allow coalescing, 614 - * allocations are done in brute force best fit. 615 - */ 616 - 617 - struct debug_alloc_header { 618 - u32 next; /* offset of next header from start of pool */ 619 - u32 size; 620 - void *caller; 621 - }; 622 - 623 - /* The memory returned by this allocator must be aligned, which means 624 - * so must the header size. Do not assume that sizeof(struct 625 - * debug_alloc_header) is a multiple of the alignment, explicitly 626 - * calculate the overhead of this header, including the alignment. 627 - * The rest of this code must not use sizeof() on any header or 628 - * pointer to a header. 629 - */ 630 - #define dah_align 8 631 - #define dah_overhead ALIGN(sizeof(struct debug_alloc_header), dah_align) 632 - 633 - static u64 debug_alloc_pool_aligned[256*1024/dah_align]; /* 256K pool */ 634 - static char *debug_alloc_pool = (char *)debug_alloc_pool_aligned; 635 - static u32 dah_first, dah_first_call = 1, dah_used, dah_used_max; 636 - 637 - /* Locking is awkward. The debug code is called from all contexts, 638 - * including non maskable interrupts. A normal spinlock is not safe 639 - * in NMI context. Try to get the debug allocator lock, if it cannot 640 - * be obtained after a second then give up. If the lock could not be 641 - * previously obtained on this cpu then only try once. 642 - * 643 - * sparse has no annotation for "this function _sometimes_ acquires a 644 - * lock", so fudge the acquire/release notation. 645 - */ 646 - static DEFINE_SPINLOCK(dap_lock); 647 - static int get_dap_lock(void) 648 - __acquires(dap_lock) 649 - { 650 - static int dap_locked = -1; 651 - int count; 652 - if (dap_locked == smp_processor_id()) 653 - count = 1; 654 - else 655 - count = 1000; 656 - while (1) { 657 - if (spin_trylock(&dap_lock)) { 658 - dap_locked = -1; 659 - return 1; 660 - } 661 - if (!count--) 662 - break; 663 - udelay(1000); 664 - } 665 - dap_locked = smp_processor_id(); 666 - __acquire(dap_lock); 667 - return 0; 668 - } 669 - 670 - void *debug_kmalloc(size_t size, gfp_t flags) 671 - { 672 - unsigned int rem, h_offset; 673 - struct debug_alloc_header *best, *bestprev, *prev, *h; 674 - void *p = NULL; 675 - if (!get_dap_lock()) { 676 - __release(dap_lock); /* we never actually got it */ 677 - return NULL; 678 - } 679 - h = (struct debug_alloc_header *)(debug_alloc_pool + dah_first); 680 - if (dah_first_call) { 681 - h->size = sizeof(debug_alloc_pool_aligned) - dah_overhead; 682 - dah_first_call = 0; 683 - } 684 - size = ALIGN(size, dah_align); 685 - prev = best = bestprev = NULL; 686 - while (1) { 687 - if (h->size >= size && (!best || h->size < best->size)) { 688 - best = h; 689 - bestprev = prev; 690 - if (h->size == size) 691 - break; 692 - } 693 - if (!h->next) 694 - break; 695 - prev = h; 696 - h = (struct debug_alloc_header *)(debug_alloc_pool + h->next); 697 - } 698 - if (!best) 699 - goto out; 700 - rem = best->size - size; 701 - /* The pool must always contain at least one header */ 702 - if (best->next == 0 && bestprev == NULL && rem < dah_overhead) 703 - goto out; 704 - if (rem >= dah_overhead) { 705 - best->size = size; 706 - h_offset = ((char *)best - debug_alloc_pool) + 707 - dah_overhead + best->size; 708 - h = (struct debug_alloc_header *)(debug_alloc_pool + h_offset); 709 - h->size = rem - dah_overhead; 710 - h->next = best->next; 711 - } else 712 - h_offset = best->next; 713 - best->caller = __builtin_return_address(0); 714 - dah_used += best->size; 715 - dah_used_max = max(dah_used, dah_used_max); 716 - if (bestprev) 717 - bestprev->next = h_offset; 718 - else 719 - dah_first = h_offset; 720 - p = (char *)best + dah_overhead; 721 - memset(p, POISON_INUSE, best->size - 1); 722 - *((char *)p + best->size - 1) = POISON_END; 723 - out: 724 - spin_unlock(&dap_lock); 725 - return p; 726 - } 727 - 728 - void debug_kfree(void *p) 729 - { 730 - struct debug_alloc_header *h; 731 - unsigned int h_offset; 732 - if (!p) 733 - return; 734 - if ((char *)p < debug_alloc_pool || 735 - (char *)p >= debug_alloc_pool + sizeof(debug_alloc_pool_aligned)) { 736 - kfree(p); 737 - return; 738 - } 739 - if (!get_dap_lock()) { 740 - __release(dap_lock); /* we never actually got it */ 741 - return; /* memory leak, cannot be helped */ 742 - } 743 - h = (struct debug_alloc_header *)((char *)p - dah_overhead); 744 - memset(p, POISON_FREE, h->size - 1); 745 - *((char *)p + h->size - 1) = POISON_END; 746 - h->caller = NULL; 747 - dah_used -= h->size; 748 - h_offset = (char *)h - debug_alloc_pool; 749 - if (h_offset < dah_first) { 750 - h->next = dah_first; 751 - dah_first = h_offset; 752 - } else { 753 - struct debug_alloc_header *prev; 754 - unsigned int prev_offset; 755 - prev = (struct debug_alloc_header *)(debug_alloc_pool + 756 - dah_first); 757 - while (1) { 758 - if (!prev->next || prev->next > h_offset) 759 - break; 760 - prev = (struct debug_alloc_header *) 761 - (debug_alloc_pool + prev->next); 762 - } 763 - prev_offset = (char *)prev - debug_alloc_pool; 764 - if (prev_offset + dah_overhead + prev->size == h_offset) { 765 - prev->size += dah_overhead + h->size; 766 - memset(h, POISON_FREE, dah_overhead - 1); 767 - *((char *)h + dah_overhead - 1) = POISON_END; 768 - h = prev; 769 - h_offset = prev_offset; 770 - } else { 771 - h->next = prev->next; 772 - prev->next = h_offset; 773 - } 774 - } 775 - if (h_offset + dah_overhead + h->size == h->next) { 776 - struct debug_alloc_header *next; 777 - next = (struct debug_alloc_header *) 778 - (debug_alloc_pool + h->next); 779 - h->size += dah_overhead + next->size; 780 - h->next = next->next; 781 - memset(next, POISON_FREE, dah_overhead - 1); 782 - *((char *)next + dah_overhead - 1) = POISON_END; 783 - } 784 - spin_unlock(&dap_lock); 785 - } 786 - 787 - void debug_kusage(void) 788 - { 789 - struct debug_alloc_header *h_free, *h_used; 790 - #ifdef CONFIG_IA64 791 - /* FIXME: using dah for ia64 unwind always results in a memory leak. 792 - * Fix that memory leak first, then set debug_kusage_one_time = 1 for 793 - * all architectures. 794 - */ 795 - static int debug_kusage_one_time; 796 - #else 797 - static int debug_kusage_one_time = 1; 798 - #endif 799 - if (!get_dap_lock()) { 800 - __release(dap_lock); /* we never actually got it */ 801 - return; 802 - } 803 - h_free = (struct debug_alloc_header *)(debug_alloc_pool + dah_first); 804 - if (dah_first == 0 && 805 - (h_free->size == sizeof(debug_alloc_pool_aligned) - dah_overhead || 806 - dah_first_call)) 807 - goto out; 808 - if (!debug_kusage_one_time) 809 - goto out; 810 - debug_kusage_one_time = 0; 811 - kdb_func_printf("debug_kmalloc memory leak dah_first %d\n", dah_first); 812 - if (dah_first) { 813 - h_used = (struct debug_alloc_header *)debug_alloc_pool; 814 - kdb_func_printf("h_used %px size %d\n", h_used, h_used->size); 815 - } 816 - do { 817 - h_used = (struct debug_alloc_header *) 818 - ((char *)h_free + dah_overhead + h_free->size); 819 - kdb_func_printf("h_used %px size %d caller %px\n", 820 - h_used, h_used->size, h_used->caller); 821 - h_free = (struct debug_alloc_header *) 822 - (debug_alloc_pool + h_free->next); 823 - } while (h_free->next); 824 - h_used = (struct debug_alloc_header *) 825 - ((char *)h_free + dah_overhead + h_free->size); 826 - if ((char *)h_used - debug_alloc_pool != 827 - sizeof(debug_alloc_pool_aligned)) 828 - kdb_func_printf("h_used %px size %d caller %px\n", 829 - h_used, h_used->size, h_used->caller); 830 - out: 831 - spin_unlock(&dap_lock); 832 657 } 833 658 834 659 /* Maintain a small stack of kdb_flags to allow recursion without disturbing