From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-9.1 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE, SPF_PASS,T_DKIMWL_WL_HIGH,URIBL_BLOCKED,USER_AGENT_GIT autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3DC59C282CE for ; Tue, 4 Jun 2019 23:32:43 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 12A62206B8 for ; Tue, 4 Jun 2019 23:32:43 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1559691163; bh=nS8p33op+lPx3xjF6fPT+mUnp7ey+7F7f1nn/Cd8jNo=; h=From:To:Cc:Subject:Date:In-Reply-To:References:List-ID:From; b=oM0m/8WJaHkUpW6KMUdiqU9fS6bzvZx7KKkqd/RO9baJjGrxBP/xRqGo8fiOTo3v5 2hT4Q778Mtcg3feMOZ/Si1fj3nx3UhwhWEeGF/ax1H8P+EC8uJx+oISk10KLoKeki7 7SZhu1B2nIlS6gtniIbuylndBeHt7n9dLjNTBYvI= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726865AbfFDXWh (ORCPT ); Tue, 4 Jun 2019 19:22:37 -0400 Received: from mail.kernel.org ([198.145.29.99]:60906 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726836AbfFDXWg (ORCPT ); Tue, 4 Jun 2019 19:22:36 -0400 Received: from sasha-vm.mshome.net (c-73-47-72-35.hsd1.nh.comcast.net [73.47.72.35]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 26EE620866; Tue, 4 Jun 2019 23:22:35 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1559690555; bh=nS8p33op+lPx3xjF6fPT+mUnp7ey+7F7f1nn/Cd8jNo=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=CmLqG8GdSB2hpVYaVEI0GuTaK+geV4sXNJ7vupghUwtO9/EiovFS/mwomUzDJOrJn c8TNLmydAg3IEgKeJXeMsHAmNYd0tsRe1k7U6JSOoCZwUQEV+iR3khRLlfkTXfhv1N HFMxUjfenkTd4B1sm74IcrJ0E9kfK79vibDMycMc= From: Sasha Levin To: linux-kernel@vger.kernel.org, stable@vger.kernel.org Cc: John Fastabend , Daniel Borkmann , Sasha Levin , netdev@vger.kernel.org, bpf@vger.kernel.org Subject: [PATCH AUTOSEL 5.1 12/60] bpf: sockmap, only stop/flush strp if it was enabled at some point Date: Tue, 4 Jun 2019 19:21:22 -0400 Message-Id: <20190604232212.6753-12-sashal@kernel.org> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20190604232212.6753-1-sashal@kernel.org> References: <20190604232212.6753-1-sashal@kernel.org> MIME-Version: 1.0 X-stable: review X-Patchwork-Hint: Ignore Content-Transfer-Encoding: 8bit Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org From: John Fastabend [ Upstream commit 014894360ec95abe868e94416b3dd6569f6e2c0c ] If we try to call strp_done on a parser that has never been initialized, because the sockmap user is only using TX side for example we get the following error. [ 883.422081] WARNING: CPU: 1 PID: 208 at kernel/workqueue.c:3030 __flush_work+0x1ca/0x1e0 ... [ 883.422095] Workqueue: events sk_psock_destroy_deferred [ 883.422097] RIP: 0010:__flush_work+0x1ca/0x1e0 This had been wrapped in a 'if (psock->parser.enabled)' logic which was broken because the strp_done() was never actually being called because we do a strp_stop() earlier in the tear down logic will set parser.enabled to false. This could result in a use after free if work was still in the queue and was resolved by the patch here, 1d79895aef18f ("sk_msg: Always cancel strp work before freeing the psock"). However, calling strp_stop(), done by the patch marked in the fixes tag, only is useful if we never initialized a strp parser program and never initialized the strp to start with. Because if we had initialized a stream parser strp_stop() would have been called by sk_psock_drop() earlier in the tear down process. By forcing the strp to stop we get past the WARNING in strp_done that checks the stopped flag but calling cancel_work_sync on work that has never been initialized is also wrong and generates the warning above. To fix check if the parser program exists. If the program exists then the strp work has been initialized and must be sync'd and cancelled before free'ing any structures. If no program exists we never initialized the stream parser in the first place so skip the sync/cancel logic implemented by strp_done. Finally, remove the strp_done its not needed and in the case where we are using the stream parser has already been called. Fixes: e8e3437762ad9 ("bpf: Stop the psock parser before canceling its work") Signed-off-by: John Fastabend Signed-off-by: Daniel Borkmann Signed-off-by: Sasha Levin --- net/core/skmsg.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/net/core/skmsg.c b/net/core/skmsg.c index cc94d921476c..49d1efa329d7 100644 --- a/net/core/skmsg.c +++ b/net/core/skmsg.c @@ -554,8 +554,10 @@ static void sk_psock_destroy_deferred(struct work_struct *gc) struct sk_psock *psock = container_of(gc, struct sk_psock, gc); /* No sk_callback_lock since already detached. */ - strp_stop(&psock->parser.strp); - strp_done(&psock->parser.strp); + + /* Parser has been stopped */ + if (psock->progs.skb_parser) + strp_done(&psock->parser.strp); cancel_work_sync(&psock->work); -- 2.20.1