From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from shards.monkeyblade.net ([184.105.139.130]:60074 "EHLO shards.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751247AbeCLPEK (ORCPT ); Mon, 12 Mar 2018 11:04:10 -0400 Date: Mon, 12 Mar 2018 11:04:08 -0400 (EDT) Message-Id: <20180312.110408.668395939190101205.davem@davemloft.net> To: lucien.xin@gmail.com Cc: netdev@vger.kernel.org, edumazet@google.com, marcelo.leitner@gmail.com, phil@nwl.cc, sd@queasysnail.net Subject: Re: [PATCHv3 net] sock_diag: request _diag module only when the family or proto has been registered From: David Miller In-Reply-To: <4e399f4124bdf8ac004235e685aa7677494d48e8.1520679470.git.lucien.xin@gmail.com> References: <4e399f4124bdf8ac004235e685aa7677494d48e8.1520679470.git.lucien.xin@gmail.com> Mime-Version: 1.0 Content-Type: Text/Plain; charset=us-ascii Content-Transfer-Encoding: 7bit Sender: netdev-owner@vger.kernel.org List-ID: From: Xin Long Date: Sat, 10 Mar 2018 18:57:50 +0800 > Now when using 'ss' in iproute, kernel would try to load all _diag > modules, which also causes corresponding family and proto modules > to be loaded as well due to module dependencies. > > Like after running 'ss', sctp, dccp, af_packet (if it works as a module) > would be loaded. > > For example: > > $ lsmod|grep sctp > $ ss > $ lsmod|grep sctp > sctp_diag 16384 0 > sctp 323584 5 sctp_diag > inet_diag 24576 4 raw_diag,tcp_diag,sctp_diag,udp_diag > libcrc32c 16384 3 nf_conntrack,nf_nat,sctp > > As these family and proto modules are loaded unintentionally, it > could cause some problems, like: > > - Some debug tools use 'ss' to collect the socket info, which loads all > those diag and family and protocol modules. It's noisy for identifying > issues. > > - Users usually expect to drop sctp init packet silently when they > have no sense of sctp protocol instead of sending abort back. > > - It wastes resources (especially with multiple netns), and SCTP module > can't be unloaded once it's loaded. > > ... > > In short, it's really inappropriate to have these family and proto > modules loaded unexpectedly when just doing debugging with inet_diag. > > This patch is to introduce sock_load_diag_module() where it loads > the _diag module only when it's corresponding family or proto has > been already registered. > > Note that we can't just load _diag module without the family or > proto loaded, as some symbols used in _diag module are from the > family or proto module. > > v1->v2: > - move inet proto check to inet_diag to avoid a compiling err. > v2->v3: > - define sock_load_diag_module in sock.c and export one symbol > only. > - improve the changelog. > > Reported-by: Sabrina Dubroca > Acked-by: Marcelo Ricardo Leitner > Acked-by: Phil Sutter > Acked-by: Sabrina Dubroca > Signed-off-by: Xin Long Ok, applied and queued up for -stable, thanks.