Received: by 2002:a05:6a10:8c0a:0:0:0:0 with SMTP id go10csp399460pxb; Wed, 3 Mar 2021 06:13:04 -0800 (PST) X-Google-Smtp-Source: ABdhPJw4c4Z0vr9Nm8CwUFSUXtB/HMxz6jiP2QHI0W4dblOj3BO0P7Nv/o93A00JIivvu1ZSXlMd X-Received: by 2002:a05:6512:2281:: with SMTP id f1mr15423611lfu.629.1614780784221; Wed, 03 Mar 2021 06:13:04 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1614780784; cv=none; d=google.com; s=arc-20160816; b=unykt1DX6mTYHezngYsjN8J7TYg0DCSrkaD3lsYPlic+PAT7AIYDz5t3Cd4ygDMNKp 9pq6SZzHXxm6cXZzKJiKArWgHZh56bVtOepQnSlVz9+nMd+myuqsp0qW38g5sDmc5ovo exBq28KYQUQa6Clf6pSjSiGkHL2AvSEoREnGBO//dk+WxVYGsR9br21X4fmgTXZMpsPe kD5MrGtULgVKL9jLMICdb4CxkAWKEghfjAtgt4Zpdt/y0nrm7Q+KNVl/b8ti9UKYYFTz Hinhc/Yv5vff5lFA7/5k9r7NBb/B8PtgGbXavPbeExnX2ZbZUHsY0rQrQxVurjIi9+2Q HXXg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :user-agent:references:in-reply-to:message-id:date:subject:cc:to :from:dkim-signature; bh=5RGJ3sBcDa0dhjSFqJXq9hf+Ph+CR9OTKgGbvAZhz+c=; b=hf++4kwxk/oRPBbfZu2Msupzw0DdPulFhCvw22BgbHFlJWBn3YNKg2T3maR/kPopjO 2PCelO9z8lFHeSNNp+SaS7H20iPb3PLwLqzl4dgzQPcib2VTYU5zd+7+IYLwfBCEPOpm 5b0n8rRFVxVlfWqzDPj08fiDz+yzAxgCURb2a1TD+rCU6g4KvhWTtBkVjKqHsjwda0Yr hLUjnHf2iE9B/w7Z+zwtDPTimZ6vOs2JHxzk5JA+LsJI/yd0t7k98DM9e2SicwhDmiEQ xNX6rnt2FK2SXtj3e3Cntybx/bBeXD7ITxXTW0Q5eIBRklY4qXY86QIcZm3Y5srJ1AcF ZHJA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linuxfoundation.org header.s=korg header.b=IvZy+9dA; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linuxfoundation.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id y8si14805959edw.240.2021.03.03.06.12.19; Wed, 03 Mar 2021 06:13:04 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@linuxfoundation.org header.s=korg header.b=IvZy+9dA; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linuxfoundation.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S243814AbhCAUs0 (ORCPT + 99 others); Mon, 1 Mar 2021 15:48:26 -0500 Received: from mail.kernel.org ([198.145.29.99]:37668 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236276AbhCARIl (ORCPT ); Mon, 1 Mar 2021 12:08:41 -0500 Received: by mail.kernel.org (Postfix) with ESMTPSA id DD9AD65012; Mon, 1 Mar 2021 16:41:53 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1614616914; bh=QL8u3tbTeb5HWu5g8dO0zy3HS3GMiT7FgTkAuBZrXwA=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=IvZy+9dAvl8dar9dIQwvRVDe0R3EaGCsN8sQyxT5RQSL0VElJB7c2BKtY7B1DYKYs zstWaqwUWZIIDNmOwRcBcGrI4UC3d8gd4VEb2gievmv9DZI7qFQ2Dj1jolvUi2o9ln 4/EewerU2tesdPt2Xpdz5+aaTOZje8ItX1KLa+us= From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , stable@vger.kernel.org, Peter Zijlstra , Josh Poimboeuf , Mathieu Desnoyers , Ingo Molnar , Alexei Starovoitov , Daniel Borkmann , Dmitry Vyukov , Martin KaFai Lau , Song Liu , Yonghong Song , Andrii Nakryiko , John Fastabend , KP Singh , netdev , bpf , Kees Cook , Florian Weimer , syzbot+83aa762ef23b6f0d1991@syzkaller.appspotmail.com, syzbot+d29e58bb557324e55e5e@syzkaller.appspotmail.com, Matt Mullins , "Steven Rostedt (VMware)" , Sasha Levin Subject: [PATCH 4.19 139/247] tracepoint: Do not fail unregistering a probe due to memory failure Date: Mon, 1 Mar 2021 17:12:39 +0100 Message-Id: <20210301161038.475315715@linuxfoundation.org> X-Mailer: git-send-email 2.30.1 In-Reply-To: <20210301161031.684018251@linuxfoundation.org> References: <20210301161031.684018251@linuxfoundation.org> User-Agent: quilt/0.66 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org From: Steven Rostedt (VMware) [ Upstream commit befe6d946551d65cddbd32b9cb0170b0249fd5ed ] The list of tracepoint callbacks is managed by an array that is protected by RCU. To update this array, a new array is allocated, the updates are copied over to the new array, and then the list of functions for the tracepoint is switched over to the new array. After a completion of an RCU grace period, the old array is freed. This process happens for both adding a callback as well as removing one. But on removing a callback, if the new array fails to be allocated, the callback is not removed, and may be used after it is freed by the clients of the tracepoint. There's really no reason to fail if the allocation for a new array fails when removing a function. Instead, the function can simply be replaced by a stub function that could be cleaned up on the next modification of the array. That is, instead of calling the function registered to the tracepoint, it would call a stub function in its place. Link: https://lore.kernel.org/r/20201115055256.65625-1-mmullins@mmlx.us Link: https://lore.kernel.org/r/20201116175107.02db396d@gandalf.local.home Link: https://lore.kernel.org/r/20201117211836.54acaef2@oasis.local.home Link: https://lkml.kernel.org/r/20201118093405.7a6d2290@gandalf.local.home [ Note, this version does use undefined compiler behavior (assuming that a stub function with no parameters or return, can be called by a location that thinks it has parameters but still no return value. Static calls do the same thing, so this trick is not without precedent. There's another solution that uses RCU tricks and is more complex, but can be an alternative if this solution becomes an issue. Link: https://lore.kernel.org/lkml/20210127170721.58bce7cc@gandalf.local.home/ ] Cc: Peter Zijlstra Cc: Josh Poimboeuf Cc: Mathieu Desnoyers Cc: Ingo Molnar Cc: Alexei Starovoitov Cc: Daniel Borkmann Cc: Dmitry Vyukov Cc: Martin KaFai Lau Cc: Song Liu Cc: Yonghong Song Cc: Andrii Nakryiko Cc: John Fastabend Cc: KP Singh Cc: netdev Cc: bpf Cc: Kees Cook Cc: Florian Weimer Fixes: 97e1c18e8d17b ("tracing: Kernel Tracepoints") Reported-by: syzbot+83aa762ef23b6f0d1991@syzkaller.appspotmail.com Reported-by: syzbot+d29e58bb557324e55e5e@syzkaller.appspotmail.com Reported-by: Matt Mullins Signed-off-by: Steven Rostedt (VMware) Tested-by: Matt Mullins Signed-off-by: Sasha Levin --- kernel/tracepoint.c | 80 ++++++++++++++++++++++++++++++++++++--------- 1 file changed, 64 insertions(+), 16 deletions(-) diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c index a3be42304485f..d5ce692319128 100644 --- a/kernel/tracepoint.c +++ b/kernel/tracepoint.c @@ -66,6 +66,12 @@ struct tp_probes { struct tracepoint_func probes[0]; }; +/* Called in removal of a func but failed to allocate a new tp_funcs */ +static void tp_stub_func(void) +{ + return; +} + static inline void *allocate_probes(int count) { struct tp_probes *p = kmalloc(count * sizeof(struct tracepoint_func) @@ -144,6 +150,7 @@ func_add(struct tracepoint_func **funcs, struct tracepoint_func *tp_func, { struct tracepoint_func *old, *new; int nr_probes = 0; + int stub_funcs = 0; int pos = -1; if (WARN_ON(!tp_func->func)) @@ -160,14 +167,34 @@ func_add(struct tracepoint_func **funcs, struct tracepoint_func *tp_func, if (old[nr_probes].func == tp_func->func && old[nr_probes].data == tp_func->data) return ERR_PTR(-EEXIST); + if (old[nr_probes].func == tp_stub_func) + stub_funcs++; } } - /* + 2 : one for new probe, one for NULL func */ - new = allocate_probes(nr_probes + 2); + /* + 2 : one for new probe, one for NULL func - stub functions */ + new = allocate_probes(nr_probes + 2 - stub_funcs); if (new == NULL) return ERR_PTR(-ENOMEM); if (old) { - if (pos < 0) { + if (stub_funcs) { + /* Need to copy one at a time to remove stubs */ + int probes = 0; + + pos = -1; + for (nr_probes = 0; old[nr_probes].func; nr_probes++) { + if (old[nr_probes].func == tp_stub_func) + continue; + if (pos < 0 && old[nr_probes].prio < prio) + pos = probes++; + new[probes++] = old[nr_probes]; + } + nr_probes = probes; + if (pos < 0) + pos = probes; + else + nr_probes--; /* Account for insertion */ + + } else if (pos < 0) { pos = nr_probes; memcpy(new, old, nr_probes * sizeof(struct tracepoint_func)); } else { @@ -201,8 +228,9 @@ static void *func_remove(struct tracepoint_func **funcs, /* (N -> M), (N > 1, M >= 0) probes */ if (tp_func->func) { for (nr_probes = 0; old[nr_probes].func; nr_probes++) { - if (old[nr_probes].func == tp_func->func && - old[nr_probes].data == tp_func->data) + if ((old[nr_probes].func == tp_func->func && + old[nr_probes].data == tp_func->data) || + old[nr_probes].func == tp_stub_func) nr_del++; } } @@ -221,14 +249,32 @@ static void *func_remove(struct tracepoint_func **funcs, /* N -> M, (N > 1, M > 0) */ /* + 1 for NULL */ new = allocate_probes(nr_probes - nr_del + 1); - if (new == NULL) - return ERR_PTR(-ENOMEM); - for (i = 0; old[i].func; i++) - if (old[i].func != tp_func->func - || old[i].data != tp_func->data) - new[j++] = old[i]; - new[nr_probes - nr_del].func = NULL; - *funcs = new; + if (new) { + for (i = 0; old[i].func; i++) + if ((old[i].func != tp_func->func + || old[i].data != tp_func->data) + && old[i].func != tp_stub_func) + new[j++] = old[i]; + new[nr_probes - nr_del].func = NULL; + *funcs = new; + } else { + /* + * Failed to allocate, replace the old function + * with calls to tp_stub_func. + */ + for (i = 0; old[i].func; i++) + if (old[i].func == tp_func->func && + old[i].data == tp_func->data) { + old[i].func = tp_stub_func; + /* Set the prio to the next event. */ + if (old[i + 1].func) + old[i].prio = + old[i + 1].prio; + else + old[i].prio = -1; + } + *funcs = old; + } } debug_print_probes(*funcs); return old; @@ -284,10 +330,12 @@ static int tracepoint_remove_func(struct tracepoint *tp, tp_funcs = rcu_dereference_protected(tp->funcs, lockdep_is_held(&tracepoints_mutex)); old = func_remove(&tp_funcs, func); - if (IS_ERR(old)) { - WARN_ON_ONCE(PTR_ERR(old) != -ENOMEM); + if (WARN_ON_ONCE(IS_ERR(old))) return PTR_ERR(old); - } + + if (tp_funcs == old) + /* Failed allocating new tp_funcs, replaced func with stub */ + return 0; if (!tp_funcs) { /* Removed last function */ -- 2.27.0