From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753904AbdKHGQt (ORCPT ); Wed, 8 Nov 2017 01:16:49 -0500 Received: from s3.sipsolutions.net ([144.76.63.242]:53998 "EHLO sipsolutions.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752932AbdKHGQr (ORCPT ); Wed, 8 Nov 2017 01:16:47 -0500 Message-ID: <1510121793.8974.1.camel@sipsolutions.net> Subject: Re: [PATCH] af_netlink: give correct bounds to dump skb for NLMSG_DONE From: Johannes Berg To: "Jason A. Donenfeld" , davem@davemloft.net, Netdev , linux-kernel@vger.kernel.org Date: Wed, 08 Nov 2017 07:16:33 +0100 In-Reply-To: <20171107112914.30576-1-Jason@zx2c4.com> References: <20171107112914.30576-1-Jason@zx2c4.com> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.26.0-1 Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 2017-11-07 at 20:29 +0900, Jason A. Donenfeld wrote: > > This patch thus reserves and restores the required length for > NLMSG_DONE during the call to the dump function. > That basically removes that space though, even when the dump isn't complete... wouldn't it be better to do something like this? diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c index f34750691c5c..fccf83598dab 100644 --- a/net/netlink/af_netlink.c +++ b/net/netlink/af_netlink.c @@ -2183,9 +2183,15 @@ static int netlink_dump(struct sock *sk) skb_reserve(skb, skb_tailroom(skb) - alloc_size); netlink_skb_set_owner_r(skb, sk); - len = cb->dump(skb, cb); + /* if the dump is already done, just complete */ + if (nlk->dump_done) + len = 0; + else + len = cb->dump(skb, cb); + + nlk->dump_done = len == 0; - if (len > 0) { + if (len > 0 || skb_tailroom(skb) < nlmsg_total_size(sizeof(len))) { mutex_unlock(nlk->cb_mutex); if (sk_filter(sk, skb)) @@ -2196,7 +2202,7 @@ static int netlink_dump(struct sock *sk) } nlh = nlmsg_put_answer(skb, cb, NLMSG_DONE, sizeof(len), NLM_F_MULTI); - if (!nlh) + if (WARN_ON(!nlh)) goto errout_skb; nl_dump_check_consistent(cb, nlh); @@ -2273,6 +2279,7 @@ int __netlink_dump_start(struct sock *ssk, struct sk_buff *skb, } nlk->cb_running = true; + nlk->dump_done = false; mutex_unlock(nlk->cb_mutex); diff --git a/net/netlink/af_netlink.h b/net/netlink/af_netlink.h index 3490f2430532..91a3652d384f 100644 --- a/net/netlink/af_netlink.h +++ b/net/netlink/af_netlink.h @@ -33,6 +33,7 @@ struct netlink_sock { wait_queue_head_t wait; bool bound; bool cb_running; + bool dump_done; struct netlink_callback cb; struct mutex *cb_mutex; struct mutex cb_def_mutex; https://p.sipsolutions.net/90574c3c0116d68a.txt (untested) johannes