| Message ID | 164915122721.982637.1510683757540074397.stgit@devnote2 (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | kprobes: rethook, ARM, arm64: Replace kretprobe trampoline with rethook | expand |
Hi Masami,
I love your patch! Perhaps something to improve:
[auto build test WARNING on bpf/master]
url: https://github.com/intel-lab-lkp/linux/commits/Masami-Hiramatsu/kprobes-rethook-ARM-arm64-Replace-kretprobe-trampoline-with-rethook/20220405-195153
base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git master
config: arm-randconfig-s032-20220406 (https://download.01.org/0day-ci/archive/20220407/202204070204.g3wpjuJi-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 11.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.4-dirty
# https://github.com/intel-lab-lkp/linux/commit/99971b0c57ce1501eda858656ed06758bbd4e376
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Masami-Hiramatsu/kprobes-rethook-ARM-arm64-Replace-kretprobe-trampoline-with-rethook/20220405-195153
git checkout 99971b0c57ce1501eda858656ed06758bbd4e376
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=arm SHELL=/bin/bash arch/arm/kernel/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
>> arch/arm/kernel/unwind.c:407:24: sparse: sparse: Using plain integer as NULL pointer
vim +407 arch/arm/kernel/unwind.c
377
378 /*
379 * Unwind a single frame starting with *sp for the symbol at *pc. It
380 * updates the *pc and *sp with the new values.
381 */
382 int unwind_frame(struct stackframe *frame)
383 {
384 const struct unwind_idx *idx;
385 struct unwind_ctrl_block ctrl;
386 unsigned long sp_low;
387
388 /* store the highest address on the stack to avoid crossing it*/
389 sp_low = frame->sp;
390 ctrl.sp_high = ALIGN(sp_low - THREAD_SIZE, THREAD_ALIGN)
391 + THREAD_SIZE;
392
393 pr_debug("%s(pc = %08lx lr = %08lx sp = %08lx)\n", __func__,
394 frame->pc, frame->lr, frame->sp);
395
396 idx = unwind_find_idx(frame->pc);
397 if (!idx) {
398 if (frame->pc && kernel_text_address(frame->pc))
399 pr_warn("unwind: Index not found %08lx\n", frame->pc);
400 return -URC_FAILURE;
401 }
402
403 ctrl.vrs[FP] = frame->fp;
404 ctrl.vrs[SP] = frame->sp;
405 ctrl.vrs[LR] = frame->lr;
406 ctrl.vrs[PC] = 0;
> 407 ctrl.lr_addr = 0;
408
409 if (idx->insn == 1)
410 /* can't unwind */
411 return -URC_FAILURE;
412 else if (frame->pc == prel31_to_addr(&idx->addr_offset)) {
413 /*
414 * Unwinding is tricky when we're halfway through the prologue,
415 * since the stack frame that the unwinder expects may not be
416 * fully set up yet. However, one thing we do know for sure is
417 * that if we are unwinding from the very first instruction of
418 * a function, we are still effectively in the stack frame of
419 * the caller, and the unwind info has no relevance yet.
420 */
421 if (frame->pc == frame->lr)
422 return -URC_FAILURE;
423 frame->pc = frame->lr;
424 return URC_OK;
425 } else if ((idx->insn & 0x80000000) == 0)
426 /* prel31 to the unwind table */
427 ctrl.insn = (unsigned long *)prel31_to_addr(&idx->insn);
428 else if ((idx->insn & 0xff000000) == 0x80000000)
429 /* only personality routine 0 supported in the index */
430 ctrl.insn = &idx->insn;
431 else {
432 pr_warn("unwind: Unsupported personality routine %08lx in the index at %p\n",
433 idx->insn, idx);
434 return -URC_FAILURE;
435 }
436
437 /* check the personality routine */
438 if ((*ctrl.insn & 0xff000000) == 0x80000000) {
439 ctrl.byte = 2;
440 ctrl.entries = 1;
441 } else if ((*ctrl.insn & 0xff000000) == 0x81000000) {
442 ctrl.byte = 1;
443 ctrl.entries = 1 + ((*ctrl.insn & 0x00ff0000) >> 16);
444 } else {
445 pr_warn("unwind: Unsupported personality routine %08lx at %p\n",
446 *ctrl.insn, ctrl.insn);
447 return -URC_FAILURE;
448 }
449
450 ctrl.check_each_pop = 0;
451
452 if (prel31_to_addr(&idx->addr_offset) == (u32)&call_with_stack) {
453 /*
454 * call_with_stack() is the only place where we permit SP to
455 * jump from one stack to another, and since we know it is
456 * guaranteed to happen, set up the SP bounds accordingly.
457 */
458 sp_low = frame->fp;
459 ctrl.sp_high = ALIGN(frame->fp, THREAD_SIZE);
460 }
461
462 while (ctrl.entries > 0) {
463 int urc;
464 if ((ctrl.sp_high - ctrl.vrs[SP]) < sizeof(ctrl.vrs))
465 ctrl.check_each_pop = 1;
466 urc = unwind_exec_insn(&ctrl);
467 if (urc < 0)
468 return urc;
469 if (ctrl.vrs[SP] < sp_low || ctrl.vrs[SP] > ctrl.sp_high)
470 return -URC_FAILURE;
471 }
472
473 if (ctrl.vrs[PC] == 0)
474 ctrl.vrs[PC] = ctrl.vrs[LR];
475
476 /* check for infinite loop */
477 if (frame->pc == ctrl.vrs[PC] && frame->sp == ctrl.vrs[SP])
478 return -URC_FAILURE;
479
480 frame->fp = ctrl.vrs[FP];
481 frame->sp = ctrl.vrs[SP];
482 frame->lr = ctrl.vrs[LR];
483 frame->pc = ctrl.vrs[PC];
484 frame->lr_addr = ctrl.lr_addr;
485
486 return URC_OK;
487 }
488
diff --git a/arch/arm/kernel/unwind.c b/arch/arm/kernel/unwind.c index a37ea6c772cd..93e767682cf4 100644 --- a/arch/arm/kernel/unwind.c +++ b/arch/arm/kernel/unwind.c @@ -404,6 +404,7 @@ int unwind_frame(struct stackframe *frame) ctrl.vrs[SP] = frame->sp; ctrl.vrs[LR] = frame->lr; ctrl.vrs[PC] = 0; + ctrl.lr_addr = 0; if (idx->insn == 1) /* can't unwind */
Since the unwind_ctrl_block::lr_addr is finally passed to stackframe::lr_addr, that value will be exposed unconditionally. Thus it should be initialized. Without this fix, when unwind_frame() doesn't update the unwind_ctrl_block::lr_addr (e.g. 'lr' register is not saved in the target function), stackframe::lr_addr will contain a wrong value. Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> --- arch/arm/kernel/unwind.c | 1 + 1 file changed, 1 insertion(+)