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
| ||
|
Date: Mon, 04 Nov 2013 17:06:05 +0900 From: Namhyung Kim <namhyung@...nel.org> To: Oleg Nesterov <oleg@...hat.com> Cc: Steven Rostedt <rostedt@...dmis.org>, Namhyung Kim <namhyung.kim@....com>, Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>, Hyeoncheol Lee <cheol.lee@....com>, Hemant Kumar <hkshaw@...ux.vnet.ibm.com>, LKML <linux-kernel@...r.kernel.org>, Srikar Dronamraju <srikar@...ux.vnet.ibm.com>, "zhangwei\(Jovi\)" <jovi.zhangwei@...wei.com>, Arnaldo Carvalho de Melo <acme@...stprotocols.net> Subject: Re: [PATCH 10/13] tracing/uprobes: Fetch args before reserving a ring buffer Hi Oleg, On Thu, 31 Oct 2013 19:16:54 +0100, Oleg Nesterov wrote: > On 10/29, Namhyung Kim wrote: >> >> @@ -630,6 +653,19 @@ probe_event_enable(struct trace_uprobe *tu, int flag, filter_func_t filter) >> if (trace_probe_is_enabled(&tu->p)) >> return -EINTR; >> >> + if (atomic_inc_return(&uprobe_buffer_ref) == 1) { >> + int cpu; >> + >> + uprobe_cpu_buffer = __alloc_percpu(PAGE_SIZE, PAGE_SIZE); >> + if (uprobe_cpu_buffer == NULL) { >> + atomic_dec(&uprobe_buffer_ref); >> + return -ENOMEM; >> + } >> + >> + for_each_possible_cpu(cpu) >> + mutex_init(&per_cpu(uprobe_cpu_mutex, cpu)); >> + } >> + >> WARN_ON(!uprobe_filter_is_empty(&tu->filter)); >> >> tu->p.flags |= flag; >> @@ -646,6 +682,11 @@ static void probe_event_disable(struct trace_uprobe *tu, int flag) >> if (!trace_probe_is_enabled(&tu->p)) >> return; >> >> + if (atomic_dec_and_test(&uprobe_buffer_ref)) { >> + free_percpu(uprobe_cpu_buffer); >> + uprobe_cpu_buffer = NULL; >> + } >> + >> WARN_ON(!uprobe_filter_is_empty(&tu->filter)); > > Do we really need atomic_t? probe_event_enable/disable is called under > event_mutex and we rely on this fact anyway. Looking at the code, it seems probe_event_enable/disable() is called without event_mutex when it called from sys_perf_event_open(). So we still need to protect refcount from concurrent accesses. > > Otherwise this logic looks racy even with atomic_t, another thread could > use the uninitialized uprobe_cpu_buffer/mutex if it registers another probe > and the handler runs before we complete the initialization, no? But yeah, this is indeed a problem. Thanks for pointing it out. I'll put a mutex to prevent such cases. Thanks, Namhyung -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists