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;
Message-ID: <ac5b9485-c9cb-9458-1101-6a219ee1faa5@marvell.com>
Date:   Fri, 30 Sep 2022 12:20:01 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.11.0
Subject: Re: [PATCH] net: prestera: acl: Add check for kmemdup
Content-Language: en-US
To:     Jiasheng Jiang <jiasheng@iscas.ac.cn>, vmytnyk@marvell.com,
        davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
        pabeni@redhat.com
Cc:     netdev@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20220930044843.32647-1-jiasheng@iscas.ac.cn>
From:   Taras Chornyi <tchornyi@marvell.com>
In-Reply-To: <20220930044843.32647-1-jiasheng@iscas.ac.cn>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?MjI3MGR1Y2tJZ1g3NDJJTHNpdi9DUkpoMjZSbWpqUmZrWTBTS1NIZWxjbDRC?=
 =?utf-8?B?S0E0NU9QaDErbS9CZDNZOUVmSlZCd1kwMGxkZDZ2dHZUQnhacElUWlhCb2dV?=
 =?utf-8?B?SVhlWmUrdHBVQnpLd3RRZ1FackxPSVBHb2F3eldmUUcwMDFocS9vR2oyWGZY?=
 =?utf-8?B?andwVmlHRHUyRjNtSDRwdWp2Z1cyY0NUR1lYWUdHN1VPM0xXOWwzaDBsT0E3?=
 =?utf-8?B?ZkIvREdhallsK3QvNFhmWHRkNE1xMmkrTHNHYzU1THU1dkV0VGJtV01RNnUy?=
 =?utf-8?B?cll6TG5ndmJ5Y2ZDa0pmbWN0eGl4TmpKTkc3Tmhvc3dnNUNEVEhsWkRjQy8r?=
 =?utf-8?B?OWgxaVRDNG1GTXk0TFJVaG84QjVPeFFsYnRPUUpuZ0R5NHBFU0YvMVF2VVc0?=
 =?utf-8?B?YjVaSjFHZVpxZ0huVDJOUEhSN0Q5b3krSGZxOEk2WVNqMEpTRjFkczRVNWY2?=
 =?utf-8?B?b0RBU3MrVDRyZXFoQm5ub0IyWVlEcU9aMDZZTHRadWd5T0xtZjhiSElRZUt5?=
 =?utf-8?B?TVJyMWVJZG96OWFpZGxJdW9VZXhwM3FMVmMrRmZwS3NQMkgvK3VScGRNYnFn?=
 =?utf-8?B?RVRSdW5aUk52Z1RFeXB0V0JuZnF5dlBBcnFWd1lZenhmNWZtOFQrYjZXbEM3?=
 =?utf-8?B?YXBhamxONUFuN1kxMmdrRkFEUjR6YmVNVXdOMjBMcWVlUWFKd3V3SytnR3F2?=
 =?utf-8?B?eXRoZTN1K2R6WURYeE8vZ0ljZTZLRjB0RDJ0dVJ2U1RxNStXSWdVdHBobUx0?=
 =?utf-8?B?WmR1Nkp5ZjRoSXVDUlkxb09Wb0JCbGYzTDc3ZzVNaTBYY2dCbWE4ZHJXdFpi?=
 =?utf-8?B?Rkk5eHBXeTJaMlBoVmcvVjk5UzhHZnB6N0RkUWx5V3g3MGJaN3NpRFRWMTEr?=
 =?utf-8?B?cjRhVzhXdlJPTjR6OWIwOFpqL1JCOW9lTkxlbkxyazdsUWxkN1ZKVk1zQmZQ?=
 =?utf-8?B?WkdJSXRaQTNSTGpTdjVROFVmYVBsVlVqMWdlNnZuK09DNXA0ZVY1cGNGY2dK?=
 =?utf-8?B?VUJaV3pISWhNRGtCcGxpSkpZcnUzTmhNc3lVZjQwdU9ENU16WGpGcXM2dU5m?=
 =?utf-8?B?aWFVdmN2WHN6TWRXZ0o5RUNweStodUgrbVFRbkd1WlJKZVZDVmJtQU9FYWlr?=
 =?utf-8?B?MTNDK1YyY1kwb25qdE1Qdnc4TEZFTmFtTU9pczl3OC9qc05TaTNPVXJ6OVhJ?=
 =?utf-8?B?cWVEelhjV1RBNUcybzFzNDIyNnU0QkFvV0xDOTZYT1FkMU9CYzhCd0pCNzdU?=
 =?utf-8?B?VkVPUWxOYk5WU1ZRYVVLdENUeStLZS9kSlFkTlhlZHdWSGFLV1B6YjBTVmh6?=
 =?utf-8?B?M3FPNTE5TVhJcXp1MkR5Ui9ZZy9ZdW1hSE5sdzNpVFRsZHByZUsyNVlWL25t?=
 =?utf-8?B?WjBEWUgrZEJjWmd2WWNhWDNGenFpWGhpL1huY2M3NmZGODJKcWVIZmxlUHRY?=
 =?utf-8?B?bkJlaGwrY3VOMmptL29OU2plQWlmcHdDaE40U1pRSlQ4OEs4a1p6eTVlOHV0?=
 =?utf-8?B?djlZOFcxeXdFMFBFQUU5Uk1TNVFQakpCajZ4NDRxd0U3Z1kveXh5ZmRQSHk4?=
 =?utf-8?B?MVBNVVN2TUtaUGx6UFVVSnVRMGprNlZOYTVlSnA0bUtqVEVxV2xad3NTc1dF?=
 =?utf-8?B?aVpGdmQ3N25xeE04czkyQWZBczRDNHI1OE5VaHRHdjkweUgyOHpCeWVseWVy?=
 =?utf-8?B?WWswckQ4Wm0rZWkrWjkrR21sVGZqQWx3OW1yamtQcDBSaUpoZE52c0UwN2Fz?=
 =?utf-8?B?TUZoOU40K1lsTi9PaEU5UVc4RCs2VXprdEx2UGYwa1grcS80djB5bXpZd05w?=
 =?utf-8?B?c1UwQkt2RWtmV2pyK2NGRG01Y0gvQTNNeUxzQXM1eC9HbHBHVlV0NlBQQ1RV?=
 =?utf-8?B?UTVUZXpYdVpDZk5qR2hCYXczZ1hRRmg0ZnFaa0x1eDg2NGVFUURLMWFpVzBX?=
 =?utf-8?B?enZmTUZMRDhqRTZ3b3hyYTlLOC9oODhtU2x4ald4Mjl6STlYMTc0cUhNMWcx?=
 =?utf-8?B?dEp0NDRHbDN6ZDgrbHlzLytVNVNjREJJVXo0RDF5ZTBjQVFJOXhvSExqVUZa?=
 =?utf-8?B?ZWlvUTVzeTdZa2RNck93eThCcHBjOXMyckpsWHFXWGJrK0xUQjJxQ0lkWG9x?=
 =?utf-8?Q?6ReqlH/cREAMdSyHswQSu8tKe?=
X-OriginatorOrg: marvell.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 70d37027-8809-4fea-b820-08daa2c4f6e2
X-MS-Exchange-CrossTenant-AuthSource: CO6PR18MB4083.namprd18.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 Sep 2022 09:20:06.8982
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 70e1fb47-1155-421d-87fc-2e58f638b6e0
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: aH4Ejf56UFlQfPsBuqKHLb9qSTUte8y4QsAoEYuT10Rq9/aktEwe7qxR/GaDY8MdQ8/EgTYK7rtsIlYbEdbEOw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH0PR18MB4307
X-Proofpoint-GUID: Cs_1H8WYYWPwW87CqtF8RlhM5PlnlE8w
X-Proofpoint-ORIG-GUID: Cs_1H8WYYWPwW87CqtF8RlhM5PlnlE8w
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.205,Aquarius:18.0.895,Hydra:6.0.528,FMLib:17.11.122.1
 definitions=2022-09-30_04,2022-09-29_03,2022-06-22_01
X-Spam-Status: No, score=-3.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,NICE_REPLY_A,RCVD_IN_DNSWL_LOW,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: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


> ----------------------------------------------------------------------
> As the kemdup could return NULL, it should be better to check the return
> value and return error if fails.
> Moreover, the return value of prestera_acl_ruleset_keymask_set() should
> be checked by cascade.
>
> Fixes: 604ba230902d ("net: prestera: flower template support")
> Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>
> ---
>   drivers/net/ethernet/marvell/prestera/prestera_acl.c    | 8 ++++++--
>   drivers/net/ethernet/marvell/prestera/prestera_acl.h    | 4 ++--
>   drivers/net/ethernet/marvell/prestera/prestera_flower.c | 6 +++++-
>   3 files changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/marvell/prestera/prestera_acl.c b/drivers/net/ethernet/marvell/prestera/prestera_acl.c
> index 3d4b85f2d541..f6b2933859d0 100644
> --- a/drivers/net/ethernet/marvell/prestera/prestera_acl.c
> +++ b/drivers/net/ethernet/marvell/prestera/prestera_acl.c
> @@ -178,10 +178,14 @@ prestera_acl_ruleset_create(struct prestera_acl *acl,
>   	return ERR_PTR(err);
>   }
>   
> -void prestera_acl_ruleset_keymask_set(struct prestera_acl_ruleset *ruleset,
> -				      void *keymask)
> +int prestera_acl_ruleset_keymask_set(struct prestera_acl_ruleset *ruleset,
> +				     void *keymask)
>   {
>   	ruleset->keymask = kmemdup(keymask, ACL_KEYMASK_SIZE, GFP_KERNEL);
> +	if (!ruleset->keymask)
> +		return -ENOMEM;
> +
> +	return 0;
>   }
>   
>   int prestera_acl_ruleset_offload(struct prestera_acl_ruleset *ruleset)
> diff --git a/drivers/net/ethernet/marvell/prestera/prestera_acl.h b/drivers/net/ethernet/marvell/prestera/prestera_acl.h
> index 03fc5b9dc925..131bfbc87cd7 100644
> --- a/drivers/net/ethernet/marvell/prestera/prestera_acl.h
> +++ b/drivers/net/ethernet/marvell/prestera/prestera_acl.h
> @@ -185,8 +185,8 @@ struct prestera_acl_ruleset *
>   prestera_acl_ruleset_lookup(struct prestera_acl *acl,
>   			    struct prestera_flow_block *block,
>   			    u32 chain_index);
> -void prestera_acl_ruleset_keymask_set(struct prestera_acl_ruleset *ruleset,
> -				      void *keymask);
> +int prestera_acl_ruleset_keymask_set(struct prestera_acl_ruleset *ruleset,
> +				     void *keymask);
>   bool prestera_acl_ruleset_is_offload(struct prestera_acl_ruleset *ruleset);
>   int prestera_acl_ruleset_offload(struct prestera_acl_ruleset *ruleset);
>   void prestera_acl_ruleset_put(struct prestera_acl_ruleset *ruleset);
> diff --git a/drivers/net/ethernet/marvell/prestera/prestera_flower.c b/drivers/net/ethernet/marvell/prestera/prestera_flower.c
> index 19d3b55c578e..cf551a8379ac 100644
> --- a/drivers/net/ethernet/marvell/prestera/prestera_flower.c
> +++ b/drivers/net/ethernet/marvell/prestera/prestera_flower.c
> @@ -452,7 +452,9 @@ int prestera_flower_tmplt_create(struct prestera_flow_block *block,
>   	}
>   
>   	/* preserve keymask/template to this ruleset */
> -	prestera_acl_ruleset_keymask_set(ruleset, rule.re_key.match.mask);
> +	err = prestera_acl_ruleset_keymask_set(ruleset, rule.re_key.match.mask);
> +	if (err)
> +		goto err_ruleset_keymask_set;
>   
>   	/* skip error, as it is not possible to reject template operation,
>   	 * so, keep the reference to the ruleset for rules to be added
> @@ -468,6 +470,8 @@ int prestera_flower_tmplt_create(struct prestera_flow_block *block,
>   	list_add_rcu(&template->list, &block->template_list);
>   	return 0;
>   
> +err_ruleset_keymask_set:
> +	prestera_acl_ruleset_put(ruleset);
>   err_ruleset_get:
>   	kfree(template);
>   err_malloc:

Reviewed-by: Taras Chornyi<tchornyi@marvell.com>