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 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 02C0CC38147 for ; Wed, 18 Jan 2023 15:09:06 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=7wjl0vGK/AqFUOtkfKB31QsDR1oWINGD9un+iNlm1xY=; b=h+GHlj+njVHjFp V1kWpzHYOsFtuYbiTyiB6ErSyqLAmkUeG6pVeUbwo0SmPcWct98MS0ZhPD+fYdmsh3IOeida2KVEG SHh7Rfe66uVkZ8PkfoL3vF83X9Zr8lMhJUwlRSxrrTS3FH1hZHMzor3587zY+qvbrdWsxp5vzGy70 eje/1LYYdxccHOfvTEcgSfRGKNw/+uxhIADKTFAsBlwbHUdql6EQs8Vfq1rblKVr9XCzzMlSFRoLz sggm8Rud4TbIcnBnvkTfdXLQy4kdRwoXgMbwpmtdUB3EPMp5qPrfA86u/vlS6nTbKrn6phNp3UGaF iv3scTt4xJfk/uGCTMBw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1pIA2k-001VBv-R9; Wed, 18 Jan 2023 15:08:02 +0000 Received: from mail-wr1-x429.google.com ([2a00:1450:4864:20::429]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1pIA2h-001VAi-Io for linux-arm-kernel@lists.infradead.org; Wed, 18 Jan 2023 15:08:01 +0000 Received: by mail-wr1-x429.google.com with SMTP id r2so34248852wrv.7 for ; 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=ZeslIe7HUpsFz6bmsbZzW4U8Hg7tqM0ZzwCq1xCjJsvWm0Fhdp6pHFDvWesWO8ufzB 3OKSD3poD4NBS2rWEFYbK8WNzyaUWIGTH02mx8zE+F8xPBzgV6M5T0dZdIajSZFKDDmz W+ZkeiBHHkDNnr2ToNxTYZFw9xH1ALKhbnzrH8Jbt0DmRg+Ku/sCOAWxoiXhqH5MGdqv TA9FRYhB9dzyqbQUq6m8eFIcQ8CTjITgOmrgBGzMA6FJSfnOtKiyYuNb9cTkjD4vK/3l UoohnfLnndxYlAMITYfDhYRepAhZzOvlW0+1Y/7XnImqOojY/iB/KFB62ocRYB1CAgW6 Fk2Q== X-Gm-Message-State: AFqh2kpuOJOr2Mpx1WiVw4MdUKokUoe6Y6JvZsnjuXpefyxvcgwusoIe 5sO9WYcA/eblPJysMLo5qb4= X-Google-Smtp-Source: AMrXdXsoMZX68p5+npbvF9kl2hMb2R16YvgJFGzIycoMffuBDklgUQz2jTwAg6qBtEwbNAj5bYYzlw== 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-Disposition: inline In-Reply-To: <87lem0w1k3.fsf@nvidia.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230118_070759_643531_6FC27494 X-CRM114-Status: GOOD ( 23.25 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel