From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S966178AbdKPRw2 (ORCPT ); Thu, 16 Nov 2017 12:52:28 -0500 Received: from mail.linuxfoundation.org ([140.211.169.12]:38096 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S965660AbdKPRu5 (ORCPT ); Thu, 16 Nov 2017 12:50:57 -0500 From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , stable@vger.kernel.org, "Jason A. Donenfeld" , Johannes Berg , "David S. Miller" Subject: [PATCH 4.13 05/44] netlink: do not set cb_running if dumps start() errs Date: Thu, 16 Nov 2017 18:42:29 +0100 Message-Id: <20171116172823.783730294@linuxfoundation.org> X-Mailer: git-send-email 2.15.0 In-Reply-To: <20171116172823.336649076@linuxfoundation.org> References: <20171116172823.336649076@linuxfoundation.org> User-Agent: quilt/0.65 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 4.13-stable review patch. If anyone has any objections, please let me know. ------------------ From: "Jason A. Donenfeld" [ Upstream commit 41c87425a1ac9b633e0fcc78eb1f19640c8fb5a0 ] It turns out that multiple places can call netlink_dump(), which means it's still possible to dereference partially initialized values in dump() that were the result of a faulty returned start(). This fixes the issue by calling start() _before_ setting cb_running to true, so that there's no chance at all of hitting the dump() function through any indirect paths. It also moves the call to start() to be when the mutex is held. This has the nice side effect of serializing invocations to start(), which is likely desirable anyway. It also prevents any possible other races that might come out of this logic. In testing this with several different pieces of tricky code to trigger these issues, this commit fixes all avenues that I'm aware of. Signed-off-by: Jason A. Donenfeld Cc: Johannes Berg Reviewed-by: Johannes Berg Signed-off-by: David S. Miller Signed-off-by: Greg Kroah-Hartman --- net/netlink/af_netlink.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) --- a/net/netlink/af_netlink.c +++ b/net/netlink/af_netlink.c @@ -2258,16 +2258,17 @@ int __netlink_dump_start(struct sock *ss cb->min_dump_alloc = control->min_dump_alloc; cb->skb = skb; + if (cb->start) { + ret = cb->start(cb); + if (ret) + goto error_unlock; + } + nlk->cb_running = true; mutex_unlock(nlk->cb_mutex); - ret = 0; - if (cb->start) - ret = cb->start(cb); - - if (!ret) - ret = netlink_dump(sk); + ret = netlink_dump(sk); sock_put(sk);