From: Masami Hiramatsu <mhiramat@kernel.org> To: kernel test robot <lkp@intel.com> Cc: stable@vger.kernel.org, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, kbuild-all@lists.01.org, linux-kernel@vger.kernel.org, Ingo Molnar <mingo@kernel.org>, Sasha Levin <sashal@kernel.org> Subject: Re: [PATCH 5.4.y] Revert "ia64: kprobes: Use generic kretprobe trampoline handler" Date: Sat, 15 Jan 2022 19:13:38 +0900 [thread overview] Message-ID: <20220115191338.b807796eb2bcf5b4c31e1314@kernel.org> (raw) In-Reply-To: <202201151231.g2sW8oWt-lkp@intel.com> 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-kprobes-Use-generic-kretprobe-trampoline-handler/20220114-182111 > 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@intel.com/config) > 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/514059de80b018e0edcf434519ff6bf41b4a519b > 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@devnote2/T/#u > 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 -- Masami Hiramatsu <mhiramat@kernel.org>
WARNING: multiple messages have this Message-ID (diff)
From: Masami Hiramatsu <mhiramat@kernel.org> To: kbuild-all@lists.01.org Subject: Re: [PATCH 5.4.y] Revert "ia64: kprobes: Use generic kretprobe trampoline handler" Date: Sat, 15 Jan 2022 19:13:38 +0900 [thread overview] Message-ID: <20220115191338.b807796eb2bcf5b4c31e1314@kernel.org> (raw) In-Reply-To: <202201151231.g2sW8oWt-lkp@intel.com> [-- Attachment #1: Type: text/plain, Size: 9670 bytes --] 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-kprobes-Use-generic-kretprobe-trampoline-handler/20220114-182111 > 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(a)intel.com/config) > 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/514059de80b018e0edcf434519ff6bf41b4a519b > 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(a)devnote2/T/#u > 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(a)lists.01.org -- Masami Hiramatsu <mhiramat@kernel.org>
next prev parent reply other threads:[~2022-01-15 10:13 UTC|newest] Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-01-12 20:15 [linux-stable-rc:linux-5.4.y 2563/9999] arch/ia64/kernel/kprobes.c:401:24: error: implicit declaration of function '__kretprobe_trampoline_handler'; did you mean 'kretprobe_trampoline'? kernel test robot 2022-01-12 20:15 ` kernel test robot 2022-01-13 9:25 ` Masami Hiramatsu 2022-01-13 9:25 ` Masami Hiramatsu 2022-01-14 7:04 ` Greg Kroah-Hartman 2022-01-14 7:04 ` Greg Kroah-Hartman 2022-01-14 7:09 ` Greg Kroah-Hartman 2022-01-14 7:09 ` Greg Kroah-Hartman 2022-01-14 7:49 ` Masami Hiramatsu 2022-01-14 7:49 ` Masami Hiramatsu 2022-01-14 10:19 ` [PATCH 5.4.y] Revert "ia64: kprobes: Use generic kretprobe trampoline handler" Masami Hiramatsu 2022-01-14 11:00 ` Greg Kroah-Hartman 2022-01-15 4:58 ` kernel test robot 2022-01-15 4:58 ` kernel test robot 2022-01-15 10:13 ` Masami Hiramatsu [this message] 2022-01-15 10:13 ` Masami Hiramatsu 2022-01-15 12:59 ` [PATCH v2 " Masami Hiramatsu 2022-01-24 15:09 ` Greg Kroah-Hartman
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20220115191338.b807796eb2bcf5b4c31e1314@kernel.org \ --to=mhiramat@kernel.org \ --cc=gregkh@linuxfoundation.org \ --cc=kbuild-all@lists.01.org \ --cc=linux-kernel@vger.kernel.org \ --cc=lkp@intel.com \ --cc=mingo@kernel.org \ --cc=sashal@kernel.org \ --cc=stable@vger.kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.