All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] kprobes: fix out-of-bounds in register_kretprobe
@ 2021-12-01  5:48 zhangyue
  2021-12-01 13:00 ` Masami Hiramatsu
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: zhangyue @ 2021-12-01  5:48 UTC (permalink / raw)
  To: naveen.n.rao, anil.s.keshavamurthy, davem, mhiramat; +Cc: linux-kernel

When the data 'rp->data_size' is negative, the code
'sizeof(struct kretprobe_instance)+rp->data_size'
is less than 'sizeof(struct kretprobe_instance)'

At this time, the pointer 'inst' may be out of
bound when it is in use.

Signed-off-by: zhangyue <zhangyue1@kylinos.cn>
---
 kernel/kprobes.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 1cf8bca1ea86..71cf6bde299f 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -1983,7 +1983,7 @@ int kprobe_on_func_entry(kprobe_opcode_t *addr, const char *sym, unsigned long o
 int register_kretprobe(struct kretprobe *rp)
 {
 	int ret;
-	struct kretprobe_instance *inst;
+	struct kretprobe_instance *inst = NULL;
 	int i;
 	void *addr;
 
@@ -2024,7 +2024,8 @@ int register_kretprobe(struct kretprobe *rp)
 
 	rp->rph->rp = rp;
 	for (i = 0; i < rp->maxactive; i++) {
-		inst = kzalloc(sizeof(struct kretprobe_instance) +
+		if (rp->data_size >= 0)
+			inst = kzalloc(sizeof(struct kretprobe_instance) +
 			       rp->data_size, GFP_KERNEL);
 		if (inst == NULL) {
 			refcount_set(&rp->rph->ref, i);
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread
* Re: [PATCH] kprobes: fix out-of-bounds in register_kretprobe
  2021-12-01  5:48 [PATCH] kprobes: fix out-of-bounds in register_kretprobe zhangyue
@ 2021-12-01 13:00 ` Masami Hiramatsu
  2021-12-01 13:06 ` Masami Hiramatsu
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Masami Hiramatsu @ 2021-12-01 13:00 UTC (permalink / raw)
  To: zhangyue; +Cc: naveen.n.rao, anil.s.keshavamurthy, davem, linux-kernel

On Wed,  1 Dec 2021 13:48:55 +0800
zhangyue <zhangyue1@kylinos.cn> wrote:

> When the data 'rp->data_size' is negative, the code
> 'sizeof(struct kretprobe_instance)+rp->data_size'
> is less than 'sizeof(struct kretprobe_instance)'
> 
> At this time, the pointer 'inst' may be out of
> bound when it is in use.

Good catch! but in that case register_kretprobe() should return -EINVAL
since there is no reason to allow minus data_size. (Thus, it must be
unsigned int, and limit with some maximum size.)

Let me fix that.

Thank you,

> 
> Signed-off-by: zhangyue <zhangyue1@kylinos.cn>
> ---
>  kernel/kprobes.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index 1cf8bca1ea86..71cf6bde299f 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -1983,7 +1983,7 @@ int kprobe_on_func_entry(kprobe_opcode_t *addr, const char *sym, unsigned long o
>  int register_kretprobe(struct kretprobe *rp)
>  {
>  	int ret;
> -	struct kretprobe_instance *inst;
> +	struct kretprobe_instance *inst = NULL;
>  	int i;
>  	void *addr;
>  
> @@ -2024,7 +2024,8 @@ int register_kretprobe(struct kretprobe *rp)
>  
>  	rp->rph->rp = rp;
>  	for (i = 0; i < rp->maxactive; i++) {
> -		inst = kzalloc(sizeof(struct kretprobe_instance) +
> +		if (rp->data_size >= 0)
> +			inst = kzalloc(sizeof(struct kretprobe_instance) +
>  			       rp->data_size, GFP_KERNEL);
>  		if (inst == NULL) {
>  			refcount_set(&rp->rph->ref, i);
> -- 
> 2.30.0
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH] kprobes: fix out-of-bounds in register_kretprobe
  2021-12-01  5:48 [PATCH] kprobes: fix out-of-bounds in register_kretprobe zhangyue
  2021-12-01 13:00 ` Masami Hiramatsu
@ 2021-12-01 13:06 ` Masami Hiramatsu
  2021-12-02  4:29   ` kernel test robot
  2021-12-05  4:26   ` kernel test robot
  3 siblings, 0 replies; 9+ messages in thread
From: Masami Hiramatsu @ 2021-12-01 13:06 UTC (permalink / raw)
  To: zhangyue; +Cc: naveen.n.rao, anil.s.keshavamurthy, davem, linux-kernel

On Wed,  1 Dec 2021 13:48:55 +0800
zhangyue <zhangyue1@kylinos.cn> wrote:

> When the data 'rp->data_size' is negative, the code
> 'sizeof(struct kretprobe_instance)+rp->data_size'
> is less than 'sizeof(struct kretprobe_instance)'

Hmm, rp->data_size is size_t, which is unsigned value.
Of course we still need some kind of maximum limitation
because if we pass enough bigger size, the 
sizeof(struct kretprobe_instance) + rp->data_size
can be negative or smaller than sizeof(struct kretprobe_instance).

Thank you,

> 
> At this time, the pointer 'inst' may be out of
> bound when it is in use.
> 
> Signed-off-by: zhangyue <zhangyue1@kylinos.cn>
> ---
>  kernel/kprobes.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index 1cf8bca1ea86..71cf6bde299f 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -1983,7 +1983,7 @@ int kprobe_on_func_entry(kprobe_opcode_t *addr, const char *sym, unsigned long o
>  int register_kretprobe(struct kretprobe *rp)
>  {
>  	int ret;
> -	struct kretprobe_instance *inst;
> +	struct kretprobe_instance *inst = NULL;
>  	int i;
>  	void *addr;
>  
> @@ -2024,7 +2024,8 @@ int register_kretprobe(struct kretprobe *rp)
>  
>  	rp->rph->rp = rp;
>  	for (i = 0; i < rp->maxactive; i++) {
> -		inst = kzalloc(sizeof(struct kretprobe_instance) +
> +		if (rp->data_size >= 0)
> +			inst = kzalloc(sizeof(struct kretprobe_instance) +
>  			       rp->data_size, GFP_KERNEL);
>  		if (inst == NULL) {
>  			refcount_set(&rp->rph->ref, i);
> -- 
> 2.30.0
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH] kprobes: fix out-of-bounds in register_kretprobe
  2021-12-01  5:48 [PATCH] kprobes: fix out-of-bounds in register_kretprobe zhangyue
@ 2021-12-02  4:29   ` kernel test robot
  2021-12-01 13:06 ` Masami Hiramatsu
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: kernel test robot @ 2021-12-02  4:29 UTC (permalink / raw)
  To: zhangyue, naveen.n.rao, anil.s.keshavamurthy, davem, mhiramat
  Cc: kbuild-all, linux-kernel

Hi zhangyue,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on rostedt-trace/for-next]
[also build test WARNING on v5.16-rc3 next-20211201]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/zhangyue/kprobes-fix-out-of-bounds-in-register_kretprobe/20211201-135046
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git for-next
config: powerpc64-randconfig-m031-20211129 (https://download.01.org/0day-ci/archive/20211202/202112021254.cDIRw2r6-lkp@intel.com/config)
compiler: powerpc64-linux-gcc (GCC) 11.2.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

smatch warnings:
kernel/kprobes.c:2107 register_kretprobe() warn: always true condition '(rp->data_size >= 0) => (0-u64max >= 0)'

vim +2107 kernel/kprobes.c

  2062	
  2063	int register_kretprobe(struct kretprobe *rp)
  2064	{
  2065		int ret;
  2066		struct kretprobe_instance *inst = NULL;
  2067		int i;
  2068		void *addr;
  2069	
  2070		ret = kprobe_on_func_entry(rp->kp.addr, rp->kp.symbol_name, rp->kp.offset);
  2071		if (ret)
  2072			return ret;
  2073	
  2074		/* If only 'rp->kp.addr' is specified, check reregistering kprobes */
  2075		if (rp->kp.addr && warn_kprobe_rereg(&rp->kp))
  2076			return -EINVAL;
  2077	
  2078		if (kretprobe_blacklist_size) {
  2079			addr = kprobe_addr(&rp->kp);
  2080			if (IS_ERR(addr))
  2081				return PTR_ERR(addr);
  2082	
  2083			for (i = 0; kretprobe_blacklist[i].name != NULL; i++) {
  2084				if (kretprobe_blacklist[i].addr == addr)
  2085					return -EINVAL;
  2086			}
  2087		}
  2088	
  2089		rp->kp.pre_handler = pre_handler_kretprobe;
  2090		rp->kp.post_handler = NULL;
  2091	
  2092		/* Pre-allocate memory for max kretprobe instances */
  2093		if (rp->maxactive <= 0) {
  2094	#ifdef CONFIG_PREEMPTION
  2095			rp->maxactive = max_t(unsigned int, 10, 2*num_possible_cpus());
  2096	#else
  2097			rp->maxactive = num_possible_cpus();
  2098	#endif
  2099		}
  2100		rp->freelist.head = NULL;
  2101		rp->rph = kzalloc(sizeof(struct kretprobe_holder), GFP_KERNEL);
  2102		if (!rp->rph)
  2103			return -ENOMEM;
  2104	
  2105		rp->rph->rp = rp;
  2106		for (i = 0; i < rp->maxactive; i++) {
> 2107			if (rp->data_size >= 0)
  2108				inst = kzalloc(sizeof(struct kretprobe_instance) +
  2109				       rp->data_size, GFP_KERNEL);
  2110			if (inst == NULL) {
  2111				refcount_set(&rp->rph->ref, i);
  2112				free_rp_inst(rp);
  2113				return -ENOMEM;
  2114			}
  2115			inst->rph = rp->rph;
  2116			freelist_add(&inst->freelist, &rp->freelist);
  2117		}
  2118		refcount_set(&rp->rph->ref, i);
  2119	
  2120		rp->nmissed = 0;
  2121		/* Establish function entry probe point */
  2122		ret = register_kprobe(&rp->kp);
  2123		if (ret != 0)
  2124			free_rp_inst(rp);
  2125		return ret;
  2126	}
  2127	EXPORT_SYMBOL_GPL(register_kretprobe);
  2128	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH] kprobes: fix out-of-bounds in register_kretprobe
@ 2021-12-02  4:29   ` kernel test robot
  0 siblings, 0 replies; 9+ messages in thread
From: kernel test robot @ 2021-12-02  4:29 UTC (permalink / raw)
  To: kbuild-all

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

Hi zhangyue,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on rostedt-trace/for-next]
[also build test WARNING on v5.16-rc3 next-20211201]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/zhangyue/kprobes-fix-out-of-bounds-in-register_kretprobe/20211201-135046
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git for-next
config: powerpc64-randconfig-m031-20211129 (https://download.01.org/0day-ci/archive/20211202/202112021254.cDIRw2r6-lkp(a)intel.com/config)
compiler: powerpc64-linux-gcc (GCC) 11.2.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

smatch warnings:
kernel/kprobes.c:2107 register_kretprobe() warn: always true condition '(rp->data_size >= 0) => (0-u64max >= 0)'

vim +2107 kernel/kprobes.c

  2062	
  2063	int register_kretprobe(struct kretprobe *rp)
  2064	{
  2065		int ret;
  2066		struct kretprobe_instance *inst = NULL;
  2067		int i;
  2068		void *addr;
  2069	
  2070		ret = kprobe_on_func_entry(rp->kp.addr, rp->kp.symbol_name, rp->kp.offset);
  2071		if (ret)
  2072			return ret;
  2073	
  2074		/* If only 'rp->kp.addr' is specified, check reregistering kprobes */
  2075		if (rp->kp.addr && warn_kprobe_rereg(&rp->kp))
  2076			return -EINVAL;
  2077	
  2078		if (kretprobe_blacklist_size) {
  2079			addr = kprobe_addr(&rp->kp);
  2080			if (IS_ERR(addr))
  2081				return PTR_ERR(addr);
  2082	
  2083			for (i = 0; kretprobe_blacklist[i].name != NULL; i++) {
  2084				if (kretprobe_blacklist[i].addr == addr)
  2085					return -EINVAL;
  2086			}
  2087		}
  2088	
  2089		rp->kp.pre_handler = pre_handler_kretprobe;
  2090		rp->kp.post_handler = NULL;
  2091	
  2092		/* Pre-allocate memory for max kretprobe instances */
  2093		if (rp->maxactive <= 0) {
  2094	#ifdef CONFIG_PREEMPTION
  2095			rp->maxactive = max_t(unsigned int, 10, 2*num_possible_cpus());
  2096	#else
  2097			rp->maxactive = num_possible_cpus();
  2098	#endif
  2099		}
  2100		rp->freelist.head = NULL;
  2101		rp->rph = kzalloc(sizeof(struct kretprobe_holder), GFP_KERNEL);
  2102		if (!rp->rph)
  2103			return -ENOMEM;
  2104	
  2105		rp->rph->rp = rp;
  2106		for (i = 0; i < rp->maxactive; i++) {
> 2107			if (rp->data_size >= 0)
  2108				inst = kzalloc(sizeof(struct kretprobe_instance) +
  2109				       rp->data_size, GFP_KERNEL);
  2110			if (inst == NULL) {
  2111				refcount_set(&rp->rph->ref, i);
  2112				free_rp_inst(rp);
  2113				return -ENOMEM;
  2114			}
  2115			inst->rph = rp->rph;
  2116			freelist_add(&inst->freelist, &rp->freelist);
  2117		}
  2118		refcount_set(&rp->rph->ref, i);
  2119	
  2120		rp->nmissed = 0;
  2121		/* Establish function entry probe point */
  2122		ret = register_kprobe(&rp->kp);
  2123		if (ret != 0)
  2124			free_rp_inst(rp);
  2125		return ret;
  2126	}
  2127	EXPORT_SYMBOL_GPL(register_kretprobe);
  2128	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH] kprobes: fix out-of-bounds in register_kretprobe
  2021-12-01  5:48 [PATCH] kprobes: fix out-of-bounds in register_kretprobe zhangyue
@ 2021-12-05  4:26   ` kernel test robot
  2021-12-01 13:06 ` Masami Hiramatsu
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: kernel test robot @ 2021-12-05  4:26 UTC (permalink / raw)
  To: zhangyue, naveen.n.rao, anil.s.keshavamurthy, davem, mhiramat
  Cc: kbuild-all, linux-kernel

Hi zhangyue,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on rostedt-trace/for-next]
[also build test WARNING on v5.16-rc3 next-20211203]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/zhangyue/kprobes-fix-out-of-bounds-in-register_kretprobe/20211201-135046
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git for-next
config: i386-randconfig-m021-20211203 (https://download.01.org/0day-ci/archive/20211205/202112051255.NQeIOpp8-lkp@intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

smatch warnings:
kernel/kprobes.c:2107 register_kretprobe() warn: always true condition '(rp->data_size >= 0) => (0-u32max >= 0)'

vim +2107 kernel/kprobes.c

  2062	
  2063	int register_kretprobe(struct kretprobe *rp)
  2064	{
  2065		int ret;
  2066		struct kretprobe_instance *inst = NULL;
  2067		int i;
  2068		void *addr;
  2069	
  2070		ret = kprobe_on_func_entry(rp->kp.addr, rp->kp.symbol_name, rp->kp.offset);
  2071		if (ret)
  2072			return ret;
  2073	
  2074		/* If only 'rp->kp.addr' is specified, check reregistering kprobes */
  2075		if (rp->kp.addr && warn_kprobe_rereg(&rp->kp))
  2076			return -EINVAL;
  2077	
  2078		if (kretprobe_blacklist_size) {
  2079			addr = kprobe_addr(&rp->kp);
  2080			if (IS_ERR(addr))
  2081				return PTR_ERR(addr);
  2082	
  2083			for (i = 0; kretprobe_blacklist[i].name != NULL; i++) {
  2084				if (kretprobe_blacklist[i].addr == addr)
  2085					return -EINVAL;
  2086			}
  2087		}
  2088	
  2089		rp->kp.pre_handler = pre_handler_kretprobe;
  2090		rp->kp.post_handler = NULL;
  2091	
  2092		/* Pre-allocate memory for max kretprobe instances */
  2093		if (rp->maxactive <= 0) {
  2094	#ifdef CONFIG_PREEMPTION
  2095			rp->maxactive = max_t(unsigned int, 10, 2*num_possible_cpus());
  2096	#else
  2097			rp->maxactive = num_possible_cpus();
  2098	#endif
  2099		}
  2100		rp->freelist.head = NULL;
  2101		rp->rph = kzalloc(sizeof(struct kretprobe_holder), GFP_KERNEL);
  2102		if (!rp->rph)
  2103			return -ENOMEM;
  2104	
  2105		rp->rph->rp = rp;
  2106		for (i = 0; i < rp->maxactive; i++) {
> 2107			if (rp->data_size >= 0)
  2108				inst = kzalloc(sizeof(struct kretprobe_instance) +
  2109				       rp->data_size, GFP_KERNEL);
  2110			if (inst == NULL) {
  2111				refcount_set(&rp->rph->ref, i);
  2112				free_rp_inst(rp);
  2113				return -ENOMEM;
  2114			}
  2115			inst->rph = rp->rph;
  2116			freelist_add(&inst->freelist, &rp->freelist);
  2117		}
  2118		refcount_set(&rp->rph->ref, i);
  2119	
  2120		rp->nmissed = 0;
  2121		/* Establish function entry probe point */
  2122		ret = register_kprobe(&rp->kp);
  2123		if (ret != 0)
  2124			free_rp_inst(rp);
  2125		return ret;
  2126	}
  2127	EXPORT_SYMBOL_GPL(register_kretprobe);
  2128	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH] kprobes: fix out-of-bounds in register_kretprobe
@ 2021-12-05  4:26   ` kernel test robot
  0 siblings, 0 replies; 9+ messages in thread
From: kernel test robot @ 2021-12-05  4:26 UTC (permalink / raw)
  To: kbuild-all

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

Hi zhangyue,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on rostedt-trace/for-next]
[also build test WARNING on v5.16-rc3 next-20211203]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/zhangyue/kprobes-fix-out-of-bounds-in-register_kretprobe/20211201-135046
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git for-next
config: i386-randconfig-m021-20211203 (https://download.01.org/0day-ci/archive/20211205/202112051255.NQeIOpp8-lkp(a)intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

smatch warnings:
kernel/kprobes.c:2107 register_kretprobe() warn: always true condition '(rp->data_size >= 0) => (0-u32max >= 0)'

vim +2107 kernel/kprobes.c

  2062	
  2063	int register_kretprobe(struct kretprobe *rp)
  2064	{
  2065		int ret;
  2066		struct kretprobe_instance *inst = NULL;
  2067		int i;
  2068		void *addr;
  2069	
  2070		ret = kprobe_on_func_entry(rp->kp.addr, rp->kp.symbol_name, rp->kp.offset);
  2071		if (ret)
  2072			return ret;
  2073	
  2074		/* If only 'rp->kp.addr' is specified, check reregistering kprobes */
  2075		if (rp->kp.addr && warn_kprobe_rereg(&rp->kp))
  2076			return -EINVAL;
  2077	
  2078		if (kretprobe_blacklist_size) {
  2079			addr = kprobe_addr(&rp->kp);
  2080			if (IS_ERR(addr))
  2081				return PTR_ERR(addr);
  2082	
  2083			for (i = 0; kretprobe_blacklist[i].name != NULL; i++) {
  2084				if (kretprobe_blacklist[i].addr == addr)
  2085					return -EINVAL;
  2086			}
  2087		}
  2088	
  2089		rp->kp.pre_handler = pre_handler_kretprobe;
  2090		rp->kp.post_handler = NULL;
  2091	
  2092		/* Pre-allocate memory for max kretprobe instances */
  2093		if (rp->maxactive <= 0) {
  2094	#ifdef CONFIG_PREEMPTION
  2095			rp->maxactive = max_t(unsigned int, 10, 2*num_possible_cpus());
  2096	#else
  2097			rp->maxactive = num_possible_cpus();
  2098	#endif
  2099		}
  2100		rp->freelist.head = NULL;
  2101		rp->rph = kzalloc(sizeof(struct kretprobe_holder), GFP_KERNEL);
  2102		if (!rp->rph)
  2103			return -ENOMEM;
  2104	
  2105		rp->rph->rp = rp;
  2106		for (i = 0; i < rp->maxactive; i++) {
> 2107			if (rp->data_size >= 0)
  2108				inst = kzalloc(sizeof(struct kretprobe_instance) +
  2109				       rp->data_size, GFP_KERNEL);
  2110			if (inst == NULL) {
  2111				refcount_set(&rp->rph->ref, i);
  2112				free_rp_inst(rp);
  2113				return -ENOMEM;
  2114			}
  2115			inst->rph = rp->rph;
  2116			freelist_add(&inst->freelist, &rp->freelist);
  2117		}
  2118		refcount_set(&rp->rph->ref, i);
  2119	
  2120		rp->nmissed = 0;
  2121		/* Establish function entry probe point */
  2122		ret = register_kprobe(&rp->kp);
  2123		if (ret != 0)
  2124			free_rp_inst(rp);
  2125		return ret;
  2126	}
  2127	EXPORT_SYMBOL_GPL(register_kretprobe);
  2128	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH] kprobes: fix out-of-bounds in register_kretprobe
  2021-12-05  4:26   ` kernel test robot
@ 2021-12-06  0:22     ` Masami Hiramatsu
  -1 siblings, 0 replies; 9+ messages in thread
From: Masami Hiramatsu @ 2021-12-06  0:22 UTC (permalink / raw)
  To: kernel test robot
  Cc: zhangyue, naveen.n.rao, anil.s.keshavamurthy, davem, mhiramat,
	kbuild-all, linux-kernel

Hi Steve,

Can you revert this patch, because as kernel-test bot says that
this does not change anything. (rp::data_size is unsigned.)

At least it should check the result of
"sizeof(struct kretprobe_instance) + rp->data_size". Moreover,
as I sent before as "kprobes: Limit max data_size of the kretprobe instances"
the data_size must be limited to avoid overflow.

Thank you,

On Sun, 5 Dec 2021 12:26:26 +0800
kernel test robot <lkp@intel.com> wrote:

> Hi zhangyue,
> 
> Thank you for the patch! Perhaps something to improve:
> 
> [auto build test WARNING on rostedt-trace/for-next]
> [also build test WARNING on v5.16-rc3 next-20211203]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
> 
> url:    https://github.com/0day-ci/linux/commits/zhangyue/kprobes-fix-out-of-bounds-in-register_kretprobe/20211201-135046
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git for-next
> config: i386-randconfig-m021-20211203 (https://download.01.org/0day-ci/archive/20211205/202112051255.NQeIOpp8-lkp@intel.com/config)
> compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> 
> smatch warnings:
> kernel/kprobes.c:2107 register_kretprobe() warn: always true condition '(rp->data_size >= 0) => (0-u32max >= 0)'
> 
> vim +2107 kernel/kprobes.c
> 
>   2062	
>   2063	int register_kretprobe(struct kretprobe *rp)
>   2064	{
>   2065		int ret;
>   2066		struct kretprobe_instance *inst = NULL;
>   2067		int i;
>   2068		void *addr;
>   2069	
>   2070		ret = kprobe_on_func_entry(rp->kp.addr, rp->kp.symbol_name, rp->kp.offset);
>   2071		if (ret)
>   2072			return ret;
>   2073	
>   2074		/* If only 'rp->kp.addr' is specified, check reregistering kprobes */
>   2075		if (rp->kp.addr && warn_kprobe_rereg(&rp->kp))
>   2076			return -EINVAL;
>   2077	
>   2078		if (kretprobe_blacklist_size) {
>   2079			addr = kprobe_addr(&rp->kp);
>   2080			if (IS_ERR(addr))
>   2081				return PTR_ERR(addr);
>   2082	
>   2083			for (i = 0; kretprobe_blacklist[i].name != NULL; i++) {
>   2084				if (kretprobe_blacklist[i].addr == addr)
>   2085					return -EINVAL;
>   2086			}
>   2087		}
>   2088	
>   2089		rp->kp.pre_handler = pre_handler_kretprobe;
>   2090		rp->kp.post_handler = NULL;
>   2091	
>   2092		/* Pre-allocate memory for max kretprobe instances */
>   2093		if (rp->maxactive <= 0) {
>   2094	#ifdef CONFIG_PREEMPTION
>   2095			rp->maxactive = max_t(unsigned int, 10, 2*num_possible_cpus());
>   2096	#else
>   2097			rp->maxactive = num_possible_cpus();
>   2098	#endif
>   2099		}
>   2100		rp->freelist.head = NULL;
>   2101		rp->rph = kzalloc(sizeof(struct kretprobe_holder), GFP_KERNEL);
>   2102		if (!rp->rph)
>   2103			return -ENOMEM;
>   2104	
>   2105		rp->rph->rp = rp;
>   2106		for (i = 0; i < rp->maxactive; i++) {
> > 2107			if (rp->data_size >= 0)
>   2108				inst = kzalloc(sizeof(struct kretprobe_instance) +
>   2109				       rp->data_size, GFP_KERNEL);
>   2110			if (inst == NULL) {
>   2111				refcount_set(&rp->rph->ref, i);
>   2112				free_rp_inst(rp);
>   2113				return -ENOMEM;
>   2114			}
>   2115			inst->rph = rp->rph;
>   2116			freelist_add(&inst->freelist, &rp->freelist);
>   2117		}
>   2118		refcount_set(&rp->rph->ref, i);
>   2119	
>   2120		rp->nmissed = 0;
>   2121		/* Establish function entry probe point */
>   2122		ret = register_kprobe(&rp->kp);
>   2123		if (ret != 0)
>   2124			free_rp_inst(rp);
>   2125		return ret;
>   2126	}
>   2127	EXPORT_SYMBOL_GPL(register_kretprobe);
>   2128	
> 
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org


-- 
Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH] kprobes: fix out-of-bounds in register_kretprobe
@ 2021-12-06  0:22     ` Masami Hiramatsu
  0 siblings, 0 replies; 9+ messages in thread
From: Masami Hiramatsu @ 2021-12-06  0:22 UTC (permalink / raw)
  To: kbuild-all

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

Hi Steve,

Can you revert this patch, because as kernel-test bot says that
this does not change anything. (rp::data_size is unsigned.)

At least it should check the result of
"sizeof(struct kretprobe_instance) + rp->data_size". Moreover,
as I sent before as "kprobes: Limit max data_size of the kretprobe instances"
the data_size must be limited to avoid overflow.

Thank you,

On Sun, 5 Dec 2021 12:26:26 +0800
kernel test robot <lkp@intel.com> wrote:

> Hi zhangyue,
> 
> Thank you for the patch! Perhaps something to improve:
> 
> [auto build test WARNING on rostedt-trace/for-next]
> [also build test WARNING on v5.16-rc3 next-20211203]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
> 
> url:    https://github.com/0day-ci/linux/commits/zhangyue/kprobes-fix-out-of-bounds-in-register_kretprobe/20211201-135046
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git for-next
> config: i386-randconfig-m021-20211203 (https://download.01.org/0day-ci/archive/20211205/202112051255.NQeIOpp8-lkp(a)intel.com/config)
> compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> 
> smatch warnings:
> kernel/kprobes.c:2107 register_kretprobe() warn: always true condition '(rp->data_size >= 0) => (0-u32max >= 0)'
> 
> vim +2107 kernel/kprobes.c
> 
>   2062	
>   2063	int register_kretprobe(struct kretprobe *rp)
>   2064	{
>   2065		int ret;
>   2066		struct kretprobe_instance *inst = NULL;
>   2067		int i;
>   2068		void *addr;
>   2069	
>   2070		ret = kprobe_on_func_entry(rp->kp.addr, rp->kp.symbol_name, rp->kp.offset);
>   2071		if (ret)
>   2072			return ret;
>   2073	
>   2074		/* If only 'rp->kp.addr' is specified, check reregistering kprobes */
>   2075		if (rp->kp.addr && warn_kprobe_rereg(&rp->kp))
>   2076			return -EINVAL;
>   2077	
>   2078		if (kretprobe_blacklist_size) {
>   2079			addr = kprobe_addr(&rp->kp);
>   2080			if (IS_ERR(addr))
>   2081				return PTR_ERR(addr);
>   2082	
>   2083			for (i = 0; kretprobe_blacklist[i].name != NULL; i++) {
>   2084				if (kretprobe_blacklist[i].addr == addr)
>   2085					return -EINVAL;
>   2086			}
>   2087		}
>   2088	
>   2089		rp->kp.pre_handler = pre_handler_kretprobe;
>   2090		rp->kp.post_handler = NULL;
>   2091	
>   2092		/* Pre-allocate memory for max kretprobe instances */
>   2093		if (rp->maxactive <= 0) {
>   2094	#ifdef CONFIG_PREEMPTION
>   2095			rp->maxactive = max_t(unsigned int, 10, 2*num_possible_cpus());
>   2096	#else
>   2097			rp->maxactive = num_possible_cpus();
>   2098	#endif
>   2099		}
>   2100		rp->freelist.head = NULL;
>   2101		rp->rph = kzalloc(sizeof(struct kretprobe_holder), GFP_KERNEL);
>   2102		if (!rp->rph)
>   2103			return -ENOMEM;
>   2104	
>   2105		rp->rph->rp = rp;
>   2106		for (i = 0; i < rp->maxactive; i++) {
> > 2107			if (rp->data_size >= 0)
>   2108				inst = kzalloc(sizeof(struct kretprobe_instance) +
>   2109				       rp->data_size, GFP_KERNEL);
>   2110			if (inst == NULL) {
>   2111				refcount_set(&rp->rph->ref, i);
>   2112				free_rp_inst(rp);
>   2113				return -ENOMEM;
>   2114			}
>   2115			inst->rph = rp->rph;
>   2116			freelist_add(&inst->freelist, &rp->freelist);
>   2117		}
>   2118		refcount_set(&rp->rph->ref, i);
>   2119	
>   2120		rp->nmissed = 0;
>   2121		/* Establish function entry probe point */
>   2122		ret = register_kprobe(&rp->kp);
>   2123		if (ret != 0)
>   2124			free_rp_inst(rp);
>   2125		return ret;
>   2126	}
>   2127	EXPORT_SYMBOL_GPL(register_kretprobe);
>   2128	
> 
> ---
> 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>

^ permalink raw reply	[flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-12-06  0:22 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-01  5:48 [PATCH] kprobes: fix out-of-bounds in register_kretprobe zhangyue
2021-12-01 13:00 ` Masami Hiramatsu
2021-12-01 13:06 ` Masami Hiramatsu
2021-12-02  4:29 ` kernel test robot
2021-12-02  4:29   ` kernel test robot
2021-12-05  4:26 ` kernel test robot
2021-12-05  4:26   ` kernel test robot
2021-12-06  0:22   ` Masami Hiramatsu
2021-12-06  0:22     ` Masami Hiramatsu

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.