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.

tracing/fprobe: Avoid kcalloc() in rcu_read_lock section

fprobe_remove_node_in_module() is called under RCU read locked, but
this invokes kcalloc() if there are more than 8 fprobes installed
on the module. Sashiko warns it because kcalloc() can sleep [1].

[1] https://sashiko.dev/#/patchset/177552432201.853249.5125045538812833325.stgit%40mhiramat.tok.corp.google.com

To fix this issue, expand the batch size to 128 and do not expand
the fprobe_addr_list, but just cancel walking on fprobe_ip_table,
update fgraph/ftrace_ops and retry the loop again.

Link: https://lore.kernel.org/all/177669367206.132053.1493637946869032744.stgit@mhiramat.tok.corp.google.com/

Fixes: 0de4c70d04a4 ("tracing: fprobe: use rhltable for fprobe_ip_table")
Cc: stable@vger.kernel.org
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>

+46 -48
+46 -48
kernel/trace/fprobe.c
··· 344 344 } 345 345 346 346 #ifdef CONFIG_MODULES 347 - static void fprobe_set_ips(unsigned long *ips, unsigned int cnt, int remove, 348 - int reset) 347 + static void fprobe_remove_ips(unsigned long *ips, unsigned int cnt) 349 348 { 350 - ftrace_set_filter_ips(&fprobe_graph_ops.ops, ips, cnt, remove, reset); 351 - ftrace_set_filter_ips(&fprobe_ftrace_ops, ips, cnt, remove, reset); 349 + ftrace_set_filter_ips(&fprobe_graph_ops.ops, ips, cnt, 1, 0); 350 + ftrace_set_filter_ips(&fprobe_ftrace_ops, ips, cnt, 1, 0); 352 351 } 353 352 #endif 354 353 #else ··· 366 367 } 367 368 368 369 #ifdef CONFIG_MODULES 369 - static void fprobe_set_ips(unsigned long *ips, unsigned int cnt, int remove, 370 - int reset) 370 + static void fprobe_remove_ips(unsigned long *ips, unsigned int cnt) 371 371 { 372 - ftrace_set_filter_ips(&fprobe_graph_ops.ops, ips, cnt, remove, reset); 372 + ftrace_set_filter_ips(&fprobe_graph_ops.ops, ips, cnt, 1, 0); 373 373 } 374 374 #endif 375 375 #endif /* !CONFIG_DYNAMIC_FTRACE_WITH_ARGS && !CONFIG_DYNAMIC_FTRACE_WITH_REGS */ ··· 540 542 541 543 #ifdef CONFIG_MODULES 542 544 543 - #define FPROBE_IPS_BATCH_INIT 8 545 + #define FPROBE_IPS_BATCH_INIT 128 544 546 /* instruction pointer address list */ 545 547 struct fprobe_addr_list { 546 548 int index; ··· 548 550 unsigned long *addrs; 549 551 }; 550 552 551 - static int fprobe_addr_list_add(struct fprobe_addr_list *alist, unsigned long addr) 552 - { 553 - unsigned long *addrs; 554 - 555 - /* Previously we failed to expand the list. */ 556 - if (alist->index == alist->size) 557 - return -ENOSPC; 558 - 559 - alist->addrs[alist->index++] = addr; 560 - if (alist->index < alist->size) 561 - return 0; 562 - 563 - /* Expand the address list */ 564 - addrs = kcalloc(alist->size * 2, sizeof(*addrs), GFP_KERNEL); 565 - if (!addrs) 566 - return -ENOMEM; 567 - 568 - memcpy(addrs, alist->addrs, alist->size * sizeof(*addrs)); 569 - alist->size *= 2; 570 - kfree(alist->addrs); 571 - alist->addrs = addrs; 572 - 573 - return 0; 574 - } 575 - 576 - static void fprobe_remove_node_in_module(struct module *mod, struct fprobe_hlist_node *node, 553 + static int fprobe_remove_node_in_module(struct module *mod, struct fprobe_hlist_node *node, 577 554 struct fprobe_addr_list *alist) 578 555 { 579 556 if (!within_module(node->addr, mod)) 580 - return; 557 + return 0; 558 + 581 559 if (delete_fprobe_node(node)) 582 - return; 583 - /* 584 - * If failed to update alist, just continue to update hlist. 585 - * Therefore, at list user handler will not hit anymore. 586 - */ 587 - fprobe_addr_list_add(alist, node->addr); 560 + return 0; 561 + /* If no address list is available, we can't track this address. */ 562 + if (!alist->addrs) 563 + return 0; 564 + 565 + alist->addrs[alist->index++] = node->addr; 566 + if (alist->index == alist->size) 567 + return -ENOSPC; 568 + return 0; 588 569 } 589 570 590 571 /* Handle module unloading to manage fprobe_ip_table. */ ··· 574 597 struct fprobe_hlist_node *node; 575 598 struct rhashtable_iter iter; 576 599 struct module *mod = data; 600 + bool retry; 577 601 578 602 if (val != MODULE_STATE_GOING) 579 603 return NOTIFY_DONE; 580 604 581 605 alist.addrs = kcalloc(alist.size, sizeof(*alist.addrs), GFP_KERNEL); 582 - /* If failed to alloc memory, we can not remove ips from hash. */ 583 - if (!alist.addrs) 584 - return NOTIFY_DONE; 606 + /* 607 + * If failed to alloc memory, ftrace_ops will not be able to remove ips from 608 + * hash, but we can still remove nodes from fprobe_ip_table, so we can avoid 609 + * the potential wrong callback. So just print a warning here and try to 610 + * continue without address list. 611 + */ 612 + WARN_ONCE(!alist.addrs, 613 + "Failed to allocate memory for fprobe_addr_list, ftrace_ops will not be updated"); 585 614 586 615 mutex_lock(&fprobe_mutex); 616 + again: 617 + retry = false; 618 + alist.index = 0; 587 619 rhltable_walk_enter(&fprobe_ip_table, &iter); 588 620 do { 589 621 rhashtable_walk_start(&iter); 590 622 591 623 while ((node = rhashtable_walk_next(&iter)) && !IS_ERR(node)) 592 - fprobe_remove_node_in_module(mod, node, &alist); 624 + if (fprobe_remove_node_in_module(mod, node, &alist) < 0) { 625 + retry = true; 626 + break; 627 + } 593 628 594 629 rhashtable_walk_stop(&iter); 595 - } while (node == ERR_PTR(-EAGAIN)); 630 + } while (node == ERR_PTR(-EAGAIN) && !retry); 596 631 rhashtable_walk_exit(&iter); 632 + /* Remove any ips from hash table(s) */ 633 + if (alist.index > 0) { 634 + fprobe_remove_ips(alist.addrs, alist.index); 635 + /* 636 + * If we break rhashtable walk loop except for -EAGAIN, we need 637 + * to restart looping from start for safety. Anyway, this is 638 + * not a hotpath. 639 + */ 640 + if (retry) 641 + goto again; 642 + } 597 643 598 - if (alist.index > 0) 599 - fprobe_set_ips(alist.addrs, alist.index, 1, 0); 600 644 mutex_unlock(&fprobe_mutex); 601 645 602 646 kfree(alist.addrs);