All of lore.kernel.org
 help / color / mirror / Atom feed
From: kernel test robot <lkp@intel.com>
To: Masami Hiramatsu <mhiramat@kernel.org>,
	stable@vger.kernel.org,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: 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 12:58:46 +0800	[thread overview]
Message-ID: <202201151231.g2sW8oWt-lkp@intel.com> (raw)
In-Reply-To: <164215559880.1662358.1475310445318313122.stgit@devnote2>

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;
         |                      ^~~~~~~~
   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

WARNING: multiple messages have this Message-ID (diff)
From: kernel test robot <lkp@intel.com>
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 12:58:46 +0800	[thread overview]
Message-ID: <202201151231.g2sW8oWt-lkp@intel.com> (raw)
In-Reply-To: <164215559880.1662358.1475310445318313122.stgit@devnote2>

[-- Attachment #1: Type: text/plain, Size: 8904 bytes --]

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;
         |                      ^~~~~~~~
   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(a)lists.01.org

  parent reply	other threads:[~2022-01-15  4:59 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 [this message]
2022-01-15  4:58           ` kernel test robot
2022-01-15 10:13           ` Masami Hiramatsu
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=202201151231.g2sW8oWt-lkp@intel.com \
    --to=lkp@intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=kbuild-all@lists.01.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mhiramat@kernel.org \
    --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: link
Be 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.