linux-kernel - Re: [PATCH v2 1/2] kprobes: propagate error from arm_kprobe

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 4 Nov 2017 06:49:55 +0900
From:   Masami Hiramatsu <mhiramat@...nel.org>
To:     Jessica Yu <jeyu@...nel.org>
Cc:     Ananth N Mavinakayanahalli <ananth@...ux.vnet.ibm.com>,
        Anil S Keshavamurthy <anil.s.keshavamurthy@...el.com>,
        "David S . Miller" <davem@...emloft.net>,
        Ingo Molnar <mingo@...nel.org>, Petr Mladek <pmladek@...e.com>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Joe Lawrence <joe.lawrence@...hat.com>,
        Jiri Kosina <jikos@...nel.org>,
        Miroslav Benes <mbenes@...e.cz>,
        Steven Rostedt <rostedt@...dmis.org>,
        live-patching@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/2] kprobes: propagate error from
 arm_kprobe_ftrace()

Hi Jessica,

Since this is under discussion about ftrace part, I just comment another one.

On Thu,  2 Nov 2017 17:33:33 +0100
Jessica Yu <jeyu@...nel.org> wrote:

[...]
> @@ -1362,9 +1372,14 @@ static int register_aggr_kprobe(struct kprobe *orig_p, struct kprobe *p)
>  
>  	if (ret == 0 && kprobe_disabled(ap) && !kprobe_disabled(p)) {
>  		ap->flags &= ~KPROBE_FLAG_DISABLED;
> -		if (!kprobes_all_disarmed)
> +		if (!kprobes_all_disarmed) {
>  			/* Arm the breakpoint again. */
> -			arm_kprobe(ap);
> +			ret = arm_kprobe(ap);
> +			if (ret) {
> +				ap->flags |= KPROBE_FLAG_DISABLED;
> +				list_del_rcu(&p->list);

You also have to wait rcu here.

> +			}
> +		}
>  	}
>  	return ret;
>  }
[...]
> @@ -2428,16 +2452,26 @@ static void arm_all_kprobes(void)
>  	/* Arming kprobes doesn't optimize kprobe itself */
>  	for (i = 0; i < KPROBE_TABLE_SIZE; i++) {
>  		head = &kprobe_table[i];
> -		hlist_for_each_entry_rcu(p, head, hlist)
> -			if (!kprobe_disabled(p))
> -				arm_kprobe(p);
> +		/* Arm all kprobes on a best-effort basis */
> +		hlist_for_each_entry_rcu(p, head, hlist) {
> +			if (!kprobe_disabled(p)) {
> +				err = arm_kprobe(p);
> +				if (err)  {
> +					errors++;
> +					ret = err;
> +				}
> +			}
> +		}
>  	}
>  
> -	printk(KERN_INFO "Kprobes globally enabled\n");
> +	if (errors)
> +		pr_warn("Kprobes globally enabled, but failed to arm %d kprobes\n", errors);

The last "kprobes" should be "probes" because it can include kretprobes :P

Thank you,

-- 
Masami Hiramatsu <mhiramat@...nel.org>