Received: by 2002:a05:6358:a55:b0:ec:fcf4:3ecf with SMTP id 21csp6656467rwb; Wed, 18 Jan 2023 07:52:07 -0800 (PST) X-Google-Smtp-Source: AMrXdXvb0I34TlqcOyCzvjUOLYCqb8QaHVJuuGHADYxtKKUc9jtvDBUvVaOTI9SOq4nQoW/Co37+ X-Received: by 2002:a17:907:8a07:b0:7c1:5ee1:4c57 with SMTP id sc7-20020a1709078a0700b007c15ee14c57mr8760321ejc.8.1674057126724; Wed, 18 Jan 2023 07:52:06 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1674057126; cv=none; d=google.com; s=arc-20160816; b=b/YAR5C1o+2LwWtv34KBYBgbNdFgz3WZOigTYqBI6GV6oCoIQYVH/thPsHVJoIXS12 EsFWQb9q018CfUajW5ORkMUgM0hQdjCrklj6l2krAffcS9w02WWCBRDlwxpAS3uv/UJX sRE6/arZkefz4Hh2Yik8PoBpH6kR81d2iKtDwO9IByLeQ70T8bh0E+NMTzylf2pD25lP 1qbxuAv6TAAlFUQYUMMoKU9Ucl0odKqrorOgV2M0l7TAIVGhPmQ4pppAGaR/pJMSNyBa 5ou9ruAwdxk25SVEcFEnNn/X/JimbxCxmCgV10w0yXPHsTmqW6Q6WeT4BPtSaKyPqV5B d7cA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:in-reply-to:content-disposition:mime-version :references:message-id:subject:cc:to:from:date:dkim-signature; bh=wVGIvPkx7V/OLAnlAwQUGA+K8l3OiY2HXWYlN9kgkPk=; b=zQzeIFmXYFXPBhoTuOMfEGWgSOixuQmFeSvQVvWEKzo135rTwf90tcLeQ2L5T9X4QH NPqizABaQD/6tBW3aCnKRjwxLxqnY3j6Phe3KkfiSeP0thv3+Avs+gSwA0NGOpJp61aA F8T83Ny0CsFvPUbjPvz1j5vos5W93B6bL2QnBV8Jobx3mPnI/RFEFbY728b0ormJWrfr GvvhwptuljPc5iA68mJP3RFfoetTvJGL7Y1dAkgeuze8yTomqQnVg26QgqgAFobLrHWW 5iIt//TdYqw/kpJ3IxQ8NyGthX5SseUUrsfPVYqO2qkz7LjFwlibMIBtCIkIa3s0Dfek U0tA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20210112 header.b=Z4pvvEXI; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Return-Path: Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id o6-20020a170906974600b008357d517d6bsi16642441ejy.394.2023.01.18.07.51.55; Wed, 18 Jan 2023 07:52:06 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20210112 header.b=Z4pvvEXI; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231296AbjARPIC (ORCPT + 45 others); Wed, 18 Jan 2023 10:08:02 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39102 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231491AbjARPHz (ORCPT ); Wed, 18 Jan 2023 10:07:55 -0500 Received: from mail-wr1-x429.google.com (mail-wr1-x429.google.com [IPv6:2a00:1450:4864:20::429]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E74111F5FE; Wed, 18 Jan 2023 07:07:54 -0800 (PST) Received: by mail-wr1-x429.google.com with SMTP id q10so14934492wrs.2; Wed, 18 Jan 2023 07:07:54 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=wVGIvPkx7V/OLAnlAwQUGA+K8l3OiY2HXWYlN9kgkPk=; b=Z4pvvEXIt1xVwj383nFpGqWWqH+8L01WcbGnS9nsDeqDWrevzPmh0AT41Z9VgSUBiY KDmhZwFZu468qosF9uaA3WDOwPxnR3hiWMwKyUna0O5YEjZx0sZGFJa69v7pWiEIz18D Eymr0Ic5qWJ6Mkq49Vy2WnPubc0u3xqgP23Plb2mQoSkPlSoaY3HAs0HV58k9T9hBmuH ASPU+07dg8jN8YwpWOrUQfPOldmB+pobKQj4TFGtAaX1yVB1hZ7SC+4qjNStWk9oi1Ap +fX+Ew9oOGcQHmUQ6YZpGfdgf7Hr52SvX0WvOAqXaY1F/lOxge5atv8ZSEEt3IJozBur DAUg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=wVGIvPkx7V/OLAnlAwQUGA+K8l3OiY2HXWYlN9kgkPk=; b=aODsReNf7eytLvDNA3R3DTEktyd2mltvoP+C3U3G4D6/lTJoTNIr7QcrctcPgawbiE 96BCoUKPzyXWtdvujvCM9VpWUyeZSXbHv8hRtY9PeGDDLAJHglqLa6OoWoO+Dmg/rnj1 mZ3SZLp/v1jfnv18DhygJ7qGK4NpKIh6SZzhmuyZvmXPxn5g4ZXBChlwF5SyqZpFp2nN DGt9Iw4/N73DqfZhjMPmoV1Q7rMBw08KKHfvO3MUoS2++Du/zs9H6BNAU2JARA46OgfS cBvGhl7nUR50+2nUC7Wq91euDUkep3mB680Kd+W6yvzjOTuJsMXdd/4lh402gIBsbGwE jT2g== X-Gm-Message-State: AFqh2krm2BFssx7loRPEyJx8u43KWR3kKXWBmK2uf6U0DFOiAAiPyvBW mDKwNlgOPYxu2sDXmW3biTI= X-Received: by 2002:a05:6000:25c:b0:2be:2bd:6817 with SMTP id m28-20020a056000025c00b002be02bd6817mr6449482wrz.2.1674054473335; Wed, 18 Jan 2023 07:07:53 -0800 (PST) Received: from localhost ([102.36.222.112]) by smtp.gmail.com with ESMTPSA id o2-20020a5d58c2000000b002bdbead763csm20679204wrf.95.2023.01.18.07.07.51 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 18 Jan 2023 07:07:52 -0800 (PST) Date: Wed, 18 Jan 2023 18:07:48 +0300 From: Dan Carpenter To: Petr Machata Cc: Daniel Machon , netdev@vger.kernel.org, davem@davemloft.net, edumazet@google.com, kuba@kernel.org, pabeni@redhat.com, lars.povlsen@microchip.com, Steen.Hegelund@microchip.com, UNGLinuxDriver@microchip.com, joe@perches.com, horatiu.vultur@microchip.com, Julia.Lawall@inria.fr, vladimir.oltean@nxp.com, maxime.chevallier@bootlin.com, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH net-next v2 3/6] net: dcb: add new rewrite table Message-ID: References: <20230116144853.2446315-1-daniel.machon@microchip.com> <20230116144853.2446315-4-daniel.machon@microchip.com> <87lem0w1k3.fsf@nvidia.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87lem0w1k3.fsf@nvidia.com> X-Spam-Status: No, score=-1.8 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_ENVFROM_END_DIGIT, FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Jan 18, 2023 at 11:54:23AM +0100, Petr Machata wrote: > > @@ -1241,6 +1242,26 @@ static int dcbnl_ieee_fill(struct sk_buff *skb, struct net_device *netdev) > > spin_unlock_bh(&dcb_lock); > > nla_nest_end(skb, app); > > > > + rewr = nla_nest_start_noflag(skb, DCB_ATTR_DCB_REWR_TABLE); > > + if (!rewr) > > + return -EMSGSIZE; > > This being new code, don't use _noflag please. > > > + > > + spin_lock_bh(&dcb_lock); > > + list_for_each_entry(itr, &dcb_rewr_list, list) { > > + if (itr->ifindex == netdev->ifindex) { > > + enum ieee_attrs_app type = > > + dcbnl_app_attr_type_get(itr->app.selector); > > + err = nla_put(skb, type, sizeof(itr->app), &itr->app); > > + if (err) { > > + spin_unlock_bh(&dcb_lock); > > This should cancel the nest started above. > > I wonder if it would be cleaner in a separate function, so that there > can be a dedicated clean-up block to goto. > > > + return -EMSGSIZE; > > + } > > + } > > + } > > + > > + spin_unlock_bh(&dcb_lock); > > + nla_nest_end(skb, rewr); If you see a bug like this, you may as well ask Julia or me to add a static checker warning for it. We're both already on the CC list but we might not be following the conversation closely... In Smatch, I thought it would be easy but it turned out I need to add a hack around to change the second nla_nest_start_noflag() from unknown to valid pointer. diff --git a/check_unwind.c b/check_unwind.c index a397afd2346b..3128476cbeb6 100644 --- a/check_unwind.c +++ b/check_unwind.c @@ -94,6 +94,11 @@ static struct ref_func_info func_table[] = { { "ieee80211_alloc_hw", ALLOC, -1, "$", &valid_ptr_min_sval, &valid_ptr_max_sval }, { "ieee80211_free_hw", RELEASE, 0, "$" }, + + { "nla_nest_start_noflag", ALLOC, 0, "$", &valid_ptr_min_sval, &valid_ptr_max_sval }, + { "nla_nest_start", ALLOC, 0, "$", &valid_ptr_min_sval, &valid_ptr_max_sval }, + { "nla_nest_end", RELEASE, 0, "$" }, + { "nla_nest_cancel", RELEASE, 0, "$" }, }; static struct smatch_state *unmatched_state(struct sm_state *sm) diff --git a/smatch_data/db/kernel.return_fixes b/smatch_data/db/kernel.return_fixes index fa4ed4ba5f0f..4782c5e10cdb 100644 --- a/smatch_data/db/kernel.return_fixes +++ b/smatch_data/db/kernel.return_fixes @@ -90,3 +90,4 @@ dma_resv_wait_timeout s64min-(-1),1-s64max 1-s64max[<=$3] mmc_io_rw_direct_host s32min-(-1),1-s32max (-4095)-(-1) ad3552r_transfer s32min-(-1),1-s32max (-4095)-(-1) adin1110_read_reg s32min-(-1),1-s32max (-4095)-(-1) +nla_nest_start_noflag 0-u64max 4096-ptr_max Unfortunately, there is something weird going on and only my unreleased version of Smatch finds the bug: net/dcb/dcbnl.c:1306 dcbnl_ieee_fill() warn: 'skb' from nla_nest_start_noflag() not released on lines: 1160,1171,1184,1197,1207,1217,1222,1232,1257. net/dcb/dcbnl.c:1502 dcbnl_cee_fill() warn: 'skb' from nla_nest_start_noflag() not released on lines: 1502. I have been working on that check recently... Both the released and unreleased versions of Smatch have the following complaints: net/dcb/dcbnl.c:400 dcbnl_getnumtcs() warn: 'skb' from nla_nest_start_noflag() not released on lines: 396. net/dcb/dcbnl.c:1061 dcbnl_build_peer_app() warn: 'skb' from nla_nest_start_noflag() not released on lines: 1061. net/dcb/dcbnl.c:1359 dcbnl_cee_pg_fill() warn: 'skb' from nla_nest_start_noflag() not released on lines: 1324,1342. regards, dan carpenter