Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1657581 > unrolled thread

[PATCH v2] signal: Avoid undefined behaviour in kill_something_info

Started byzhongjiang <zhongjiang@huawei.com>
First post2017-06-05 15:00 +0200
Last post2017-06-05 15:50 +0200
Articles 5 — 4 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH v2] signal: Avoid undefined behaviour in kill_something_info zhongjiang <zhongjiang@huawei.com> - 2017-06-05 15:00 +0200
    Re: [PATCH v2] signal: Avoid undefined behaviour in  kill_something_info Michal Hocko <mhocko@kernel.org> - 2017-06-05 15:10 +0200
      Re: [PATCH v2] signal: Avoid undefined behaviour in kill_something_info zhong jiang <zhongjiang@huawei.com> - 2017-06-05 15:30 +0200
    Re: [PATCH v2] signal: Avoid undefined behaviour in  kill_something_info Oleg Nesterov <oleg@redhat.com> - 2017-06-05 15:40 +0200
      Re: [PATCH v2] signal: Avoid undefined behaviour in kill_something_info zhong jiang <zhongjiang@huawei.com> - 2017-06-05 15:50 +0200

#1657581 — [PATCH v2] signal: Avoid undefined behaviour in kill_something_info

Fromzhongjiang <zhongjiang@huawei.com>
Date2017-06-05 15:00 +0200
Subject[PATCH v2] signal: Avoid undefined behaviour in kill_something_info
Message-ID<tOZLY-30O-35@gated-at.bofh.it>
when I run the kill(72057458746458112, 0) in the userspace, I hit
the following issue.

[  304.606353] UBSAN: Undefined behaviour in kernel/signal.c:1462:11
[  304.612622] negation of -2147483648 cannot be represented in type 'int':
[  304.619516] CPU: 226 PID: 9849 Comm: test Tainted: G    B          ---- -------   3.10.0-327.53.58.70.x86_64_ubsan+ #116
[  304.630692] Hardware name: Huawei Technologies Co., Ltd. RH8100 V3/BC61PBIA, BIOS BLHSV028 11/11/2014
[  304.640168]  ffffffff825ded30 000000005dc276fa ffff883c3a4b7ce0 ffffffff81d6eb06
[  304.647870]  ffff883c3a4b7cf8 ffffffff81d6ebb9 ffffffff825ded20 ffff883c3a4b7de8
[  304.655584]  ffffffff81d6fc89 0000000041b58ab3 ffffffff8228d6d8 ffffffff81d6fb80
[  304.663299] Call Trace:
[  304.665827]  [<ffffffff81d6eb06>] dump_stack+0x19/0x1b
[  304.671115]  [<ffffffff81d6ebb9>] ubsan_epilogue+0xd/0x50
[  304.676668]  [<ffffffff81d6fc89>] __ubsan_handle_negate_overflow+0x109/0x14e
[  304.683917]  [<ffffffff81d6fb80>] ? __ubsan_handle_divrem_overflow+0x1df/0x1df
[  304.691353]  [<ffffffff8134a129>] ? __inc_zone_state+0x29/0xf0
[  304.697358]  [<ffffffff813272df>] ? __lru_cache_add+0x8f/0xe0
[  304.703272]  [<ffffffff8132764e>] ? lru_cache_add+0xe/0x10
[  304.708921]  [<ffffffff812263bd>] ? map_id_up+0xad/0xe0
[  304.714306]  [<ffffffff8113126e>] SYSC_kill+0x43e/0x4d0
[  304.725359]  [<ffffffff8116e630>] ? lg_local_unlock+0x20/0xd0
[  304.736978]  [<ffffffff81130e30>] ? kill_pid+0x20/0x20
[  304.747928]  [<ffffffff81366f90>] ? __pmd_alloc+0x180/0x180
[  304.759273]  [<ffffffff8143f80b>] ? mntput+0x3b/0x70
[  304.769919]  [<ffffffff81d85c3c>] ? __do_page_fault+0x2bc/0x650
[  304.781462]  [<ffffffff8123bb47>] ? __audit_syscall_entry+0x1f7/0x2a0
[  304.793476]  [<ffffffff8113535e>] SyS_kill+0xe/0x10
[  304.803859]  [<ffffffff81d91109>] system_call_fastpath+0x16/0x1b

The patch add particular case to avoid the UBSAN detection.

Signed-off-by: zhongjiang <zhongjiang@huawei.com>
---
 kernel/signal.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/kernel/signal.c b/kernel/signal.c
index ca92bcf..63148f7 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -1395,6 +1395,12 @@ static int kill_something_info(int sig, struct siginfo *info, pid_t pid)
 
 	read_lock(&tasklist_lock);
 	if (pid != -1) {
+		/*
+	 	 * -INT_MIN is undefined, it need to exclude following case to 
+ 		 * avoid the UBSAN detection.
+		 */
+		if (pid == INT_MIN)
+			return -ESRCH;
 		ret = __kill_pgrp_info(sig, info,
 				pid ? find_vpid(-pid) : task_pgrp(current));
 	} else {
-- 
1.7.12.4

[toc] | [next] | [standalone]

#1657585 — Re: [PATCH v2] signal: Avoid undefined behaviour in kill_something_info

FromMichal Hocko <mhocko@kernel.org>
Date2017-06-05 15:10 +0200
SubjectRe: [PATCH v2] signal: Avoid undefined behaviour in kill_something_info
Message-ID<tOZVE-3jm-23@gated-at.bofh.it>
In reply to#1657581
On Mon 05-06-17 20:53:27, zhongjiang wrote:
> diff --git a/kernel/signal.c b/kernel/signal.c
> index ca92bcf..63148f7 100644
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -1395,6 +1395,12 @@ static int kill_something_info(int sig, struct siginfo *info, pid_t pid)
>  
>  	read_lock(&tasklist_lock);
>  	if (pid != -1) {
> +		/*
> +	 	 * -INT_MIN is undefined, it need to exclude following case to 
> + 		 * avoid the UBSAN detection.
> +		 */
> +		if (pid == INT_MIN)
> +			return -ESRCH;

this will obviously keep the tasklist_lock held...

>  		ret = __kill_pgrp_info(sig, info,
>  				pid ? find_vpid(-pid) : task_pgrp(current));
>  	} else {
> -- 
> 1.7.12.4

-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1657593

Fromzhong jiang <zhongjiang@huawei.com>
Date2017-06-05 15:30 +0200
Message-ID<tP0eZ-3pX-9@gated-at.bofh.it>
In reply to#1657585
On 2017/6/5 21:09, Michal Hocko wrote:
> On Mon 05-06-17 20:53:27, zhongjiang wrote:
>> diff --git a/kernel/signal.c b/kernel/signal.c
>> index ca92bcf..63148f7 100644
>> --- a/kernel/signal.c
>> +++ b/kernel/signal.c
>> @@ -1395,6 +1395,12 @@ static int kill_something_info(int sig, struct siginfo *info, pid_t pid)
>>  
>>  	read_lock(&tasklist_lock);
>>  	if (pid != -1) {
>> +		/*
>> +	 	 * -INT_MIN is undefined, it need to exclude following case to 
>> + 		 * avoid the UBSAN detection.
>> +		 */
>> +		if (pid == INT_MIN)
>> +			return -ESRCH;
> this will obviously keep the tasklist_lock held...
 oh, it is my fault.   Thank you for clarify.

 Thanks
zhongjiang
>>  		ret = __kill_pgrp_info(sig, info,
>>  				pid ? find_vpid(-pid) : task_pgrp(current));
>>  	} else {
>> -- 
>> 1.7.12.4

[toc] | [prev] | [next] | [standalone]

#1657597 — Re: [PATCH v2] signal: Avoid undefined behaviour in kill_something_info

FromOleg Nesterov <oleg@redhat.com>
Date2017-06-05 15:40 +0200
SubjectRe: [PATCH v2] signal: Avoid undefined behaviour in kill_something_info
Message-ID<tP0oF-3td-7@gated-at.bofh.it>
In reply to#1657581
On 06/05, zhongjiang wrote:
>
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -1395,6 +1395,12 @@ static int kill_something_info(int sig, struct siginfo *info, pid_t pid)
>
>  	read_lock(&tasklist_lock);
>  	if (pid != -1) {
> +		/*
> +	 	 * -INT_MIN is undefined, it need to exclude following case to
> + 		 * avoid the UBSAN detection.
> +		 */
> +		if (pid == INT_MIN)
> +			return -ESRCH;

you need to do this before read_lock(tasklist)

Oleg.

[toc] | [prev] | [next] | [standalone]

#1657606

Fromzhong jiang <zhongjiang@huawei.com>
Date2017-06-05 15:50 +0200
Message-ID<tP0ym-3wI-9@gated-at.bofh.it>
In reply to#1657597
On 2017/6/5 21:31, Oleg Nesterov wrote:
> On 06/05, zhongjiang wrote:
>> --- a/kernel/signal.c
>> +++ b/kernel/signal.c
>> @@ -1395,6 +1395,12 @@ static int kill_something_info(int sig, struct siginfo *info, pid_t pid)
>>
>>  	read_lock(&tasklist_lock);
>>  	if (pid != -1) {
>> +		/*
>> +	 	 * -INT_MIN is undefined, it need to exclude following case to
>> + 		 * avoid the UBSAN detection.
>> +		 */
>> +		if (pid == INT_MIN)
>> +			return -ESRCH;
> you need to do this before read_lock(tasklist)
>
> Oleg.
>
>
> .
>
  I am so sorry for disturbing.

 Thanks
zhongjiang

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web