This reverts commit 77fa5e15c933a1ec812de61ad709c00aa51e96ae.
Since the upstream commit e792ff804f49720ce003b3e4c618b5d996256a18 depends on the generic kretprobe trampoline handler, which was introduced by commit 66ada2ccae4e ("kprobes: Add generic kretprobe trampoline handler") but that is not ported to the stable kernel because it is not a bugfix series. So revert this commit to fix a build error.
NOTE: I keep commit a7fe2378454c ("ia64: kprobes: Fix to pass correct trampoline address to the handler") on the tree, that seems just a cleanup without the original reverted commit, but it would be better to use dereference_function_descriptor() macro instead of accessing descriptor's field directly.
Fixes: 77fa5e15c933 ("ia64: kprobes: Use generic kretprobe trampoline handler") Reported-by: kernel test robot lkp@intel.com Signed-off-by: Masami Hiramatsu mhiramat@kernel.org --- arch/ia64/kernel/kprobes.c | 78 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 75 insertions(+), 3 deletions(-)
diff --git a/arch/ia64/kernel/kprobes.c b/arch/ia64/kernel/kprobes.c index 8a223d0e4918..5d2d58644378 100644 --- a/arch/ia64/kernel/kprobes.c +++ b/arch/ia64/kernel/kprobes.c @@ -396,10 +396,83 @@ static void kretprobe_trampoline(void) { }
+/* + * At this point the target function has been tricked into + * returning into our trampoline. Lookup the associated instance + * and then: + * - call the handler function + * - cleanup by marking the instance as unused + * - long jump back to the original return address + */ int __kprobes trampoline_probe_handler(struct kprobe *p, struct pt_regs *regs) { - regs->cr_iip = __kretprobe_trampoline_handler(regs, - dereference_function_descriptor(kretprobe_trampoline), NULL); + struct kretprobe_instance *ri = NULL; + struct hlist_head *head, empty_rp; + struct hlist_node *tmp; + unsigned long flags, orig_ret_address = 0; + unsigned long trampoline_address = + dereference_function_descriptor(kretprobe_trampoline); + + INIT_HLIST_HEAD(&empty_rp); + kretprobe_hash_lock(current, &head, &flags); + + /* + * It is possible to have multiple instances associated with a given + * task either because an multiple functions in the call path + * have a return probe installed on them, and/or more than one return + * return probe was registered for a target function. + * + * We can handle this because: + * - instances are always inserted at the head of the list + * - when multiple return probes are registered for the same + * function, the first instance's ret_addr will point to the + * real return address, and all the rest will point to + * kretprobe_trampoline + */ + hlist_for_each_entry_safe(ri, tmp, head, hlist) { + if (ri->task != current) + /* another task is sharing our hash bucket */ + continue; + + orig_ret_address = (unsigned long)ri->ret_addr; + if (orig_ret_address != trampoline_address) + /* + * This is the real return address. Any other + * instances associated with this task are for + * other calls deeper on the call stack + */ + break; + } + + regs->cr_iip = orig_ret_address; + + hlist_for_each_entry_safe(ri, tmp, head, hlist) { + if (ri->task != current) + /* another task is sharing our hash bucket */ + continue; + + if (ri->rp && ri->rp->handler) + ri->rp->handler(ri, regs); + + orig_ret_address = (unsigned long)ri->ret_addr; + recycle_rp_inst(ri, &empty_rp); + + if (orig_ret_address != trampoline_address) + /* + * This is the real return address. Any other + * instances associated with this task are for + * other calls deeper on the call stack + */ + break; + } + kretprobe_assert(ri, orig_ret_address, trampoline_address); + + kretprobe_hash_unlock(current, &flags); + + hlist_for_each_entry_safe(ri, tmp, &empty_rp, hlist) { + hlist_del(&ri->hlist); + kfree(ri); + } /* * By returning a non-zero value, we are telling * kprobe_handler() that we don't want the post_handler @@ -412,7 +485,6 @@ void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri, struct pt_regs *regs) { ri->ret_addr = (kprobe_opcode_t *)regs->b0; - ri->fp = NULL;
/* Replace the return addr with trampoline addr */ regs->b0 = (unsigned long)dereference_function_descriptor(kretprobe_trampoline);
On Fri, Jan 14, 2022 at 07:19:59PM +0900, Masami Hiramatsu wrote:
This reverts commit 77fa5e15c933a1ec812de61ad709c00aa51e96ae.
Since the upstream commit e792ff804f49720ce003b3e4c618b5d996256a18 depends on the generic kretprobe trampoline handler, which was introduced by commit 66ada2ccae4e ("kprobes: Add generic kretprobe trampoline handler") but that is not ported to the stable kernel because it is not a bugfix series. So revert this commit to fix a build error.
NOTE: I keep commit a7fe2378454c ("ia64: kprobes: Fix to pass correct trampoline address to the handler") on the tree, that seems just a cleanup without the original reverted commit, but it would be better to use dereference_function_descriptor() macro instead of accessing descriptor's field directly.
Fixes: 77fa5e15c933 ("ia64: kprobes: Use generic kretprobe trampoline handler") Reported-by: kernel test robot lkp@intel.com Signed-off-by: Masami Hiramatsu mhiramat@kernel.org
arch/ia64/kernel/kprobes.c | 78 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 75 insertions(+), 3 deletions(-)
Thanks for this, I'll queue it up after this round of stable releases are out.
greg k-h
Hi Masami,
I love your patch! Perhaps something to improve:
[auto build test WARNING on stable/linux-5.4.y]
url: https://github.com/0day-ci/linux/commits/Masami-Hiramatsu/Revert-ia64-kprobe... base: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git linux-5.4.y config: ia64-allmodconfig (https://download.01.org/0day-ci/archive/20220115/202201151231.g2sW8oWt-lkp@i...) compiler: ia64-linux-gcc (GCC) 11.2.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/514059de80b018e0edcf434519ff6bf41b4a... git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Masami-Hiramatsu/Revert-ia64-kprobes-Use-generic-kretprobe-trampoline-handler/20220114-182111 git checkout 514059de80b018e0edcf434519ff6bf41b4a519b # save the config file to linux build tree mkdir build_dir COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=ia64 SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot lkp@intel.com
All warnings (new ones prefixed by >>):
arch/ia64/kernel/kprobes.c: In function 'get_kprobe_inst': arch/ia64/kernel/kprobes.c:325:22: warning: variable 'template' set but not used [-Wunused-but-set-variable] 325 | unsigned int template; | ^~~~~~~~ arch/ia64/kernel/kprobes.c: At top level: arch/ia64/kernel/kprobes.c:407:15: warning: no previous prototype for 'trampoline_probe_handler' [-Wmissing-prototypes] 407 | int __kprobes trampoline_probe_handler(struct kprobe *p, struct pt_regs *regs) | ^~~~~~~~~~~~~~~~~~~~~~~~ arch/ia64/kernel/kprobes.c: In function 'trampoline_probe_handler':
arch/ia64/kernel/kprobes.c:414:17: warning: initialization of 'long unsigned int' from 'void *' makes integer from pointer without a cast [-Wint-conversion]
414 | dereference_function_descriptor(kretprobe_trampoline); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Kconfig warnings: (for reference only) WARNING: unmet direct dependencies detected for FRAME_POINTER Depends on DEBUG_KERNEL && (M68K || UML || SUPERH) || ARCH_WANT_FRAME_POINTERS Selected by - FAULT_INJECTION_STACKTRACE_FILTER && FAULT_INJECTION_DEBUG_FS && STACKTRACE_SUPPORT && !X86_64 && !MIPS && !PPC && !S390 && !MICROBLAZE && !ARM && !ARC && !X86
vim +414 arch/ia64/kernel/kprobes.c
320 321 static void __kprobes get_kprobe_inst(bundle_t *bundle, uint slot, 322 unsigned long *kprobe_inst, uint *major_opcode) 323 { 324 unsigned long kprobe_inst_p0, kprobe_inst_p1;
325 unsigned int template;
326 327 template = bundle->quad0.template; 328 329 switch (slot) { 330 case 0: 331 *major_opcode = (bundle->quad0.slot0 >> SLOT0_OPCODE_SHIFT); 332 *kprobe_inst = bundle->quad0.slot0; 333 break; 334 case 1: 335 *major_opcode = (bundle->quad1.slot1_p1 >> SLOT1_p1_OPCODE_SHIFT); 336 kprobe_inst_p0 = bundle->quad0.slot1_p0; 337 kprobe_inst_p1 = bundle->quad1.slot1_p1; 338 *kprobe_inst = kprobe_inst_p0 | (kprobe_inst_p1 << (64-46)); 339 break; 340 case 2: 341 *major_opcode = (bundle->quad1.slot2 >> SLOT2_OPCODE_SHIFT); 342 *kprobe_inst = bundle->quad1.slot2; 343 break; 344 } 345 } 346 347 /* Returns non-zero if the addr is in the Interrupt Vector Table */ 348 static int __kprobes in_ivt_functions(unsigned long addr) 349 { 350 return (addr >= (unsigned long)__start_ivt_text 351 && addr < (unsigned long)__end_ivt_text); 352 } 353 354 static int __kprobes valid_kprobe_addr(int template, int slot, 355 unsigned long addr) 356 { 357 if ((slot > 2) || ((bundle_encoding[template][1] == L) && slot > 1)) { 358 printk(KERN_WARNING "Attempting to insert unaligned kprobe " 359 "at 0x%lx\n", addr); 360 return -EINVAL; 361 } 362 363 if (in_ivt_functions(addr)) { 364 printk(KERN_WARNING "Kprobes can't be inserted inside " 365 "IVT functions at 0x%lx\n", addr); 366 return -EINVAL; 367 } 368 369 return 0; 370 } 371 372 static void __kprobes save_previous_kprobe(struct kprobe_ctlblk *kcb) 373 { 374 unsigned int i; 375 i = atomic_add_return(1, &kcb->prev_kprobe_index); 376 kcb->prev_kprobe[i-1].kp = kprobe_running(); 377 kcb->prev_kprobe[i-1].status = kcb->kprobe_status; 378 } 379 380 static void __kprobes restore_previous_kprobe(struct kprobe_ctlblk *kcb) 381 { 382 unsigned int i; 383 i = atomic_read(&kcb->prev_kprobe_index); 384 __this_cpu_write(current_kprobe, kcb->prev_kprobe[i-1].kp); 385 kcb->kprobe_status = kcb->prev_kprobe[i-1].status; 386 atomic_sub(1, &kcb->prev_kprobe_index); 387 } 388 389 static void __kprobes set_current_kprobe(struct kprobe *p, 390 struct kprobe_ctlblk *kcb) 391 { 392 __this_cpu_write(current_kprobe, p); 393 } 394 395 static void kretprobe_trampoline(void) 396 { 397 } 398 399 /* 400 * At this point the target function has been tricked into 401 * returning into our trampoline. Lookup the associated instance 402 * and then: 403 * - call the handler function 404 * - cleanup by marking the instance as unused 405 * - long jump back to the original return address 406 */ 407 int __kprobes trampoline_probe_handler(struct kprobe *p, struct pt_regs *regs) 408 { 409 struct kretprobe_instance *ri = NULL; 410 struct hlist_head *head, empty_rp; 411 struct hlist_node *tmp; 412 unsigned long flags, orig_ret_address = 0; 413 unsigned long trampoline_address =
414 dereference_function_descriptor(kretprobe_trampoline);
415 416 INIT_HLIST_HEAD(&empty_rp); 417 kretprobe_hash_lock(current, &head, &flags); 418 419 /* 420 * It is possible to have multiple instances associated with a given 421 * task either because an multiple functions in the call path 422 * have a return probe installed on them, and/or more than one return 423 * return probe was registered for a target function. 424 * 425 * We can handle this because: 426 * - instances are always inserted at the head of the list 427 * - when multiple return probes are registered for the same 428 * function, the first instance's ret_addr will point to the 429 * real return address, and all the rest will point to 430 * kretprobe_trampoline 431 */ 432 hlist_for_each_entry_safe(ri, tmp, head, hlist) { 433 if (ri->task != current) 434 /* another task is sharing our hash bucket */ 435 continue; 436 437 orig_ret_address = (unsigned long)ri->ret_addr; 438 if (orig_ret_address != trampoline_address) 439 /* 440 * This is the real return address. Any other 441 * instances associated with this task are for 442 * other calls deeper on the call stack 443 */ 444 break; 445 } 446 447 regs->cr_iip = orig_ret_address; 448 449 hlist_for_each_entry_safe(ri, tmp, head, hlist) { 450 if (ri->task != current) 451 /* another task is sharing our hash bucket */ 452 continue; 453 454 if (ri->rp && ri->rp->handler) 455 ri->rp->handler(ri, regs); 456 457 orig_ret_address = (unsigned long)ri->ret_addr; 458 recycle_rp_inst(ri, &empty_rp); 459 460 if (orig_ret_address != trampoline_address) 461 /* 462 * This is the real return address. Any other 463 * instances associated with this task are for 464 * other calls deeper on the call stack 465 */ 466 break; 467 } 468 kretprobe_assert(ri, orig_ret_address, trampoline_address); 469 470 kretprobe_hash_unlock(current, &flags); 471 472 hlist_for_each_entry_safe(ri, tmp, &empty_rp, hlist) { 473 hlist_del(&ri->hlist); 474 kfree(ri); 475 } 476 /* 477 * By returning a non-zero value, we are telling 478 * kprobe_handler() that we don't want the post_handler 479 * to run (and have re-enabled preemption) 480 */ 481 return 1; 482 } 483
--- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
On Sat, 15 Jan 2022 12:58:46 +0800 kernel test robot lkp@intel.com wrote:
Hi Masami,
I love your patch! Perhaps something to improve:
[auto build test WARNING on stable/linux-5.4.y]
url: https://github.com/0day-ci/linux/commits/Masami-Hiramatsu/Revert-ia64-kprobe... base: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git linux-5.4.y config: ia64-allmodconfig (https://download.01.org/0day-ci/archive/20220115/202201151231.g2sW8oWt-lkp@i...) compiler: ia64-linux-gcc (GCC) 11.2.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/514059de80b018e0edcf434519ff6bf41b4a... git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Masami-Hiramatsu/Revert-ia64-kprobes-Use-generic-kretprobe-trampoline-handler/20220114-182111 git checkout 514059de80b018e0edcf434519ff6bf41b4a519b # save the config file to linux build tree mkdir build_dir COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=ia64 SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot lkp@intel.com
All warnings (new ones prefixed by >>):
arch/ia64/kernel/kprobes.c: In function 'get_kprobe_inst': arch/ia64/kernel/kprobes.c:325:22: warning: variable 'template' set but not used [-Wunused-but-set-variable] 325 | unsigned int template; | ^~~~~~~~
This one is fixed by recent patch.
https://lore.kernel.org/all/164208575387.1590449.8278421820882450166.stgit@d...
arch/ia64/kernel/kprobes.c: At top level: arch/ia64/kernel/kprobes.c:407:15: warning: no previous prototype for 'trampoline_probe_handler' [-Wmissing-prototypes] 407 | int __kprobes trampoline_probe_handler(struct kprobe *p, struct pt_regs *regs) | ^~~~~~~~~~~~~~~~~~~~~~~~ arch/ia64/kernel/kprobes.c: In function 'trampoline_probe_handler':
arch/ia64/kernel/kprobes.c:414:17: warning: initialization of 'long unsigned int' from 'void *' makes integer from pointer without a cast [-Wint-conversion]
414 | dereference_function_descriptor(kretprobe_trampoline); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Oops. I forgot to cast it. I'll resend the patch.
Thank you,
Kconfig warnings: (for reference only) WARNING: unmet direct dependencies detected for FRAME_POINTER Depends on DEBUG_KERNEL && (M68K || UML || SUPERH) || ARCH_WANT_FRAME_POINTERS Selected by
- FAULT_INJECTION_STACKTRACE_FILTER && FAULT_INJECTION_DEBUG_FS && STACKTRACE_SUPPORT && !X86_64 && !MIPS && !PPC && !S390 && !MICROBLAZE && !ARM && !ARC && !X86
vim +414 arch/ia64/kernel/kprobes.c
320 321 static void __kprobes get_kprobe_inst(bundle_t *bundle, uint slot, 322 unsigned long *kprobe_inst, uint *major_opcode) 323 { 324 unsigned long kprobe_inst_p0, kprobe_inst_p1;
325 unsigned int template;
326 327 template = bundle->quad0.template; 328 329 switch (slot) { 330 case 0: 331 *major_opcode = (bundle->quad0.slot0 >> SLOT0_OPCODE_SHIFT); 332 *kprobe_inst = bundle->quad0.slot0; 333 break; 334 case 1: 335 *major_opcode = (bundle->quad1.slot1_p1 >> SLOT1_p1_OPCODE_SHIFT); 336 kprobe_inst_p0 = bundle->quad0.slot1_p0; 337 kprobe_inst_p1 = bundle->quad1.slot1_p1; 338 *kprobe_inst = kprobe_inst_p0 | (kprobe_inst_p1 << (64-46)); 339 break; 340 case 2: 341 *major_opcode = (bundle->quad1.slot2 >> SLOT2_OPCODE_SHIFT); 342 *kprobe_inst = bundle->quad1.slot2; 343 break; 344 } 345 } 346 347 /* Returns non-zero if the addr is in the Interrupt Vector Table */ 348 static int __kprobes in_ivt_functions(unsigned long addr) 349 { 350 return (addr >= (unsigned long)__start_ivt_text 351 && addr < (unsigned long)__end_ivt_text); 352 } 353 354 static int __kprobes valid_kprobe_addr(int template, int slot, 355 unsigned long addr) 356 { 357 if ((slot > 2) || ((bundle_encoding[template][1] == L) && slot > 1)) { 358 printk(KERN_WARNING "Attempting to insert unaligned kprobe " 359 "at 0x%lx\n", addr); 360 return -EINVAL; 361 } 362 363 if (in_ivt_functions(addr)) { 364 printk(KERN_WARNING "Kprobes can't be inserted inside " 365 "IVT functions at 0x%lx\n", addr); 366 return -EINVAL; 367 } 368 369 return 0; 370 } 371 372 static void __kprobes save_previous_kprobe(struct kprobe_ctlblk *kcb) 373 { 374 unsigned int i; 375 i = atomic_add_return(1, &kcb->prev_kprobe_index); 376 kcb->prev_kprobe[i-1].kp = kprobe_running(); 377 kcb->prev_kprobe[i-1].status = kcb->kprobe_status; 378 } 379 380 static void __kprobes restore_previous_kprobe(struct kprobe_ctlblk *kcb) 381 { 382 unsigned int i; 383 i = atomic_read(&kcb->prev_kprobe_index); 384 __this_cpu_write(current_kprobe, kcb->prev_kprobe[i-1].kp); 385 kcb->kprobe_status = kcb->prev_kprobe[i-1].status; 386 atomic_sub(1, &kcb->prev_kprobe_index); 387 } 388 389 static void __kprobes set_current_kprobe(struct kprobe *p, 390 struct kprobe_ctlblk *kcb) 391 { 392 __this_cpu_write(current_kprobe, p); 393 } 394 395 static void kretprobe_trampoline(void) 396 { 397 } 398 399 /* 400 * At this point the target function has been tricked into 401 * returning into our trampoline. Lookup the associated instance 402 * and then: 403 * - call the handler function 404 * - cleanup by marking the instance as unused 405 * - long jump back to the original return address 406 */ 407 int __kprobes trampoline_probe_handler(struct kprobe *p, struct pt_regs *regs) 408 { 409 struct kretprobe_instance *ri = NULL; 410 struct hlist_head *head, empty_rp; 411 struct hlist_node *tmp; 412 unsigned long flags, orig_ret_address = 0; 413 unsigned long trampoline_address =
414 dereference_function_descriptor(kretprobe_trampoline);
415 416 INIT_HLIST_HEAD(&empty_rp); 417 kretprobe_hash_lock(current, &head, &flags); 418 419 /* 420 * It is possible to have multiple instances associated with a given 421 * task either because an multiple functions in the call path 422 * have a return probe installed on them, and/or more than one return 423 * return probe was registered for a target function. 424 * 425 * We can handle this because: 426 * - instances are always inserted at the head of the list 427 * - when multiple return probes are registered for the same 428 * function, the first instance's ret_addr will point to the 429 * real return address, and all the rest will point to 430 * kretprobe_trampoline 431 */ 432 hlist_for_each_entry_safe(ri, tmp, head, hlist) { 433 if (ri->task != current) 434 /* another task is sharing our hash bucket */ 435 continue; 436 437 orig_ret_address = (unsigned long)ri->ret_addr; 438 if (orig_ret_address != trampoline_address) 439 /* 440 * This is the real return address. Any other 441 * instances associated with this task are for 442 * other calls deeper on the call stack 443 */ 444 break; 445 } 446 447 regs->cr_iip = orig_ret_address; 448 449 hlist_for_each_entry_safe(ri, tmp, head, hlist) { 450 if (ri->task != current) 451 /* another task is sharing our hash bucket */ 452 continue; 453 454 if (ri->rp && ri->rp->handler) 455 ri->rp->handler(ri, regs); 456 457 orig_ret_address = (unsigned long)ri->ret_addr; 458 recycle_rp_inst(ri, &empty_rp); 459 460 if (orig_ret_address != trampoline_address) 461 /* 462 * This is the real return address. Any other 463 * instances associated with this task are for 464 * other calls deeper on the call stack 465 */ 466 break; 467 } 468 kretprobe_assert(ri, orig_ret_address, trampoline_address); 469 470 kretprobe_hash_unlock(current, &flags); 471 472 hlist_for_each_entry_safe(ri, tmp, &empty_rp, hlist) { 473 hlist_del(&ri->hlist); 474 kfree(ri); 475 } 476 /* 477 * By returning a non-zero value, we are telling 478 * kprobe_handler() that we don't want the post_handler 479 * to run (and have re-enabled preemption) 480 */ 481 return 1; 482 } 483
0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
linux-stable-mirror@lists.linaro.org