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;
Date:   Tue, 6 Jun 2023 10:53:55 +0200
From:   Larysa Zaremba <larysa.zaremba@intel.com>
To:     Hangyu Hua <hbh25y@gmail.com>
CC:     <jhs@mojatatu.com>, <xiyou.wangcong@gmail.com>, <jiri@resnulli.us>,
        <davem@davemloft.net>, <edumazet@google.com>, <kuba@kernel.org>,
        <pabeni@redhat.com>, <netdev@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH net] net: sched: fix possible refcount leak in
 tc_chain_tmplt_add()
Message-ID: <ZH70I+yV66OpMxbo@lincoln>
References: <20230605070158.48403-1-hbh25y@gmail.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20230605070158.48403-1-hbh25y@gmail.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?7fM/OvcPhw1PHUO3drzhc0denhR4ozCXTBDFx0xjNd00AVspjTrKll5R4oPH?=
 =?us-ascii?Q?jyvEBpFwnWWpq5Il1iK76FDihw4+JvaDcCUJ8XRpECbhFnitnYVEsGtBD4an?=
 =?us-ascii?Q?nDtb7l7hp3KeRa3ze9dHnlYsc+RjE/9XkEFriFsMTX100OP+0DY2jGS1E/Ft?=
 =?us-ascii?Q?1WnJ/cEQ3nmPMXyewDTg5K8gpOMjrZfCFH8bA3vt5eqkaXyyy5+RghRIThDT?=
 =?us-ascii?Q?wjg+RTnPla/VBEiw9Tz5SxBGfdMdWkhkhULg5gO5/qqD8J5YJDDnKE8xvnu7?=
 =?us-ascii?Q?4f/WAM4N5jm79od+Qkor6Gl1CxcpOsjlikXAg9z4wDShqFN3EDnFrO+gNDfa?=
 =?us-ascii?Q?F8tCNaUzY/MiePf62limBccr01xwMppmuW77vG3NVT/c5iEcH4Ctm/5Ls3CK?=
 =?us-ascii?Q?h2PWlvF+ijBPciKF2qSUVULVKoP+0fwZP4PUcD2iV54MwPOwE0ohGD5HkBW5?=
 =?us-ascii?Q?mK0cjj1jaVzS4mE7JJ7OaWuCDlVABNzg/SDl2xcwFBgGO2V0tjI9eoEleryT?=
 =?us-ascii?Q?XG2+q14FG0dKxeakCW6WlHERbwoizBZXMPl2byeufTNsGqocQ7inUZgJbJy0?=
 =?us-ascii?Q?qXRz6ZN6s9JOn1MG0UnxH/VTq3yl4qJdTpBIfOm3+2vyOa7GD8T4V5mg7zhM?=
 =?us-ascii?Q?3tgCTdeyyUt7A/SgZ3ewLS8yrJaHvOfxCe1HwGJbak93LFwNXWX4VzzyiZla?=
 =?us-ascii?Q?cmhB8IFY1nlej9JG311UbLGfIlgoGKYv6j79e91agmvt7eV8UXaCd/5ADWxH?=
 =?us-ascii?Q?8TuHqonfrsZiEfaXUqfhI2tjKsQKGNayctpufO6gZDCAQ/XJXDydvD+rQLeJ?=
 =?us-ascii?Q?MUhLv+9NS8OxcimffZhZFeTTE0wSGmABWqIy4ZMzjleOMmQfUsYv+/SFVuXb?=
 =?us-ascii?Q?KdB/V2GBvYjkPs+DT/g/UirN0T1G3sBWbUjeYO5G6UaLtlI/xjOPrc/4VbWk?=
 =?us-ascii?Q?4xUyAKuzFdRegDOzv48W3U3BqSJZeOtPPbRfFtxN9Yih52ii5Ebi/jsvL9LP?=
 =?us-ascii?Q?OccPmYeXBwQ03nIZG5eKj0HLLRsqTeJhjDtSX1zs1JMMooRoh1UXQ8jSyDM1?=
 =?us-ascii?Q?pu2ouc6oGpQdk32tcB0oFuM/zukN+CU1EAWYtfTN0pvVRzm6vcGCv4ZaafkO?=
 =?us-ascii?Q?UGxPDAWnZMfNkKuKBNeTjQOi5yOHPLGy94DrRyBYfQ0PaQMysA+cPdJ6kWi/?=
 =?us-ascii?Q?SPNzTqT6GuaIwe1d7kMfYKWHvMErlWDC0acmBh57bM4DwTJme/Ub/XcV6+RH?=
 =?us-ascii?Q?h9utvMXgx1r+Cfw8nqhelm8jLS6wHuC3LyopWecokKWDpdM5bRp5qI3Ob4s5?=
 =?us-ascii?Q?GUhyKxGBX+sNf7+5s+TfZ1LR82mHiqv5nkalPMVC88vNmB1AumqwaSQA4lcq?=
 =?us-ascii?Q?q4//bR0iaWuuuDNOyHT6SGcM5W72yiYlVVBF6iyfxhRK4y4cYzzzGd1yGIry?=
 =?us-ascii?Q?0tmjDxHHWBO7ZRcmZ8HY6DtLeXhx/nU9tzhfOP5aQr+lUGfTevLxPHvVtF24?=
 =?us-ascii?Q?LhAM5haoX7KiIORQfGDHMq5U+/1suxtwmdUcwbv3FVtIOZqTMuwVngLZ1pli?=
 =?us-ascii?Q?gKfy/wLkLXE8DhD5j6b/N8pCmZBN4yzhWAZG+BPJHrw86z6JkqNUWP8K8rJB?=
 =?us-ascii?Q?vA=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: bcd016b1-702c-49d8-5789-08db666bf63e
X-MS-Exchange-CrossTenant-AuthSource: SN7PR11MB7540.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Jun 2023 08:56:48.3187
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: myivvvkRKNqBrgtNVJ2jk7CTqusJsgp45WJk1yWevmtqqZI+LZiqI0DoNX2QQc8Mg1PPC9tGT1afV/BL5KIhwpRiwUBQ/NgJJsmDff7t7/A=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA2PR11MB5083
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED
        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: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Mon, Jun 05, 2023 at 03:01:58PM +0800, Hangyu Hua wrote:
> try_module_get can be called in tcf_proto_lookup_ops. So if ops don't
> implement the corresponding function we should call module_put to drop
> the refcount.
> 

Code seems reasonable. But commit message is pretty hard to understand.
Please, replace "corresponding" with "required".
Also change the first sentence, do not use "can". From what I see, successful
execution of tcf_proto_lookup_ops always means we now hold reference to module.

CC me in v2, I'll give you Reviewed-by.

> Fixes: 9f407f1768d3 ("net: sched: introduce chain templates")
> Signed-off-by: Hangyu Hua <hbh25y@gmail.com>
> ---
>  net/sched/cls_api.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
> index 2621550bfddc..92bfb892e638 100644
> --- a/net/sched/cls_api.c
> +++ b/net/sched/cls_api.c
> @@ -2952,6 +2952,7 @@ static int tc_chain_tmplt_add(struct tcf_chain *chain, struct net *net,
>  		return PTR_ERR(ops);
>  	if (!ops->tmplt_create || !ops->tmplt_destroy || !ops->tmplt_dump) {
>  		NL_SET_ERR_MSG(extack, "Chain templates are not supported with specified classifier");
> +		module_put(ops->owner);
>  		return -EOPNOTSUPP;
>  	}
>  
> -- 
> 2.34.1
> 
>