Date:   Thu, 9 Mar 2023 17:01:17 +0100
From:   Simon Horman <simon.horman@corigine.com>
To:     Daniil Tatianin <d-tatianin@yandex-team.ru>
Cc:     Ariel Elior <aelior@marvell.com>,
        Manish Chopra <manishc@marvell.com>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Yuval Mintz <Yuval.Mintz@qlogic.com>, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v1] qed/qed_dev: guard against a possible division by zero
Message-ID: <ZAoCzbwBBVfP/bno@corigine.com>
References: <20230309125636.176337-1-d-tatianin@yandex-team.ru>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230309125636.176337-1-d-tatianin@yandex-team.ru>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?OKIK9EtOUokrKh/VMil7trz8Sw/KUph+Zx/5QOpqvkeOjeE7GvMEpDoIXIub?=
 =?us-ascii?Q?Dfrj29RGN60x3HhlaBJ39OSOtZeafxVNPZycB3yjdhw8hMRRCR2xI29AaNmh?=
 =?us-ascii?Q?V8k+lhI/hRJYpZ2SAG4SsSRI+dxiVrWDqmSumGlKgQZrsjxUODELaJtbFjkZ?=
 =?us-ascii?Q?31zVBnKE2iaaEpwjRt/pzzV8h9tSFnyUTICu71QOl89jivh5hbvJKoHT4Szp?=
 =?us-ascii?Q?AegUBB3QDSRrHLyoQWa0cwp1cLikhxMPwrHtYRiVBEJ0MkJO9plirIZ7ofrn?=
 =?us-ascii?Q?MhgtGLJCEtaRZW9dsAIKz39YNwllR1L9snc4J54MNW4ZbD9rd1/e+2IvcUlL?=
 =?us-ascii?Q?1YiNt3ivUhzRbK5jdXjFp1oR/aBEzYWgJN9QzrLyeGhm1UwwdGwLInjxQnvE?=
 =?us-ascii?Q?I+PRylYNbkLoWHkSjegy4DpWMsBD4zy+Zqo3iB6gS/RxMfpW3Rs9VoFBxx9G?=
 =?us-ascii?Q?LmgeXQOFAQm7ia7IxLlGfFBahCs7geLi/IfNAcNKHVBxGasEodjRQ5iG6z6J?=
 =?us-ascii?Q?suzjIHz+/WfntiT5VLXg7RCbO2vQH+O60e902wolHnNR0aAbqBxhg0woMWyc?=
 =?us-ascii?Q?AIzH2dz5jT4K2mhI1kx/6m+MPJgiCIYucIA9o8ckm/dbHkFOFcXigU1DAjlC?=
 =?us-ascii?Q?2wU3l2gJzND9dv8rkfzSZvCv7aTdp/oY4NygsasEQp7nYk3hcuKHVeGz10LM?=
 =?us-ascii?Q?L0kxnVzffbv9rDMlaKd+kJoALW9RodMgE++Cc2xLEIPy50jJZtQU+PUFK0FT?=
 =?us-ascii?Q?uR8XkmKLnbuodlwORMFmvrGMQz5x3sCnfyYQiUcJ+zl/eBhS88jX8MJH0MOS?=
 =?us-ascii?Q?v5+o0qbao54VyljmwyQEwDLgu9d7o+2L1Vj1cGS0u3uDpazBmn68oEVVds9g?=
 =?us-ascii?Q?eA35uhcfAeGTglNc3JP3qOxxrjCHOu6YAy8SvNOl6jqz9ar1lByjiApyODgs?=
 =?us-ascii?Q?7ufh4alekmtzig0wfCnRLbUR467b4gULiFlBufpEwt7h2aB4dBhQ58KDTQi6?=
 =?us-ascii?Q?KqHS+0oMasL2cxaQcz+qJI5+7GGmdpqfbLBMnvZxFs1lPv+lFpDE2wwTYwuQ?=
 =?us-ascii?Q?H0EGCfmL6sstjiF+FKMwZaD7R15uqqkVbx6sJziUbH07qLnkyK+hb5Ronisx?=
 =?us-ascii?Q?FqGaVyLxY1z28/P4sdmgLMYXLKPoNzouNB3uxUOHVK33h5EGJoZRYk2+Yp48?=
 =?us-ascii?Q?4I4/DrnFumAczSEGFJON7iQVG0YfKvkVZvONURhRP1b7QDE48IlHB4S8QUt0?=
 =?us-ascii?Q?a8zRrR4GKGvyp6mvj9inHVDtCg7ckWbiN8SFXo1zasMqR72GZCQsWNJP7ypm?=
 =?us-ascii?Q?PZioI1ZG1tOFKi1jqzFuBEQq+iy5TKh7bjM6R+tL/JvWfyGk7qg12wxbdVNk?=
 =?us-ascii?Q?rvgxasxpdrlCTqnPW1m4eibMhF+RPXW6r5yxUn4DpcgcWuF2BzO8cz8AFsiX?=
 =?us-ascii?Q?U1MGAYKdIfEauAlsOGRMBLmISIITGWskFIKHvZkbfQ9H2DQlfdCMu5xiA0ht?=
 =?us-ascii?Q?77Ml6HZjMulMXQhU6ZNjkU7chquYz2X/CCzDggfQnqDIcq43NoDJHIzDjMNR?=
 =?us-ascii?Q?2z15nqcnirX4N6EggnkuoA707cEQjwygDB35fwAVbiKcaRJXeVwaDrhwE34K?=
 =?us-ascii?Q?eT7hAIVriI+q6vXwxKLbDX54TmucQqBd27wfc+LxOTSNuUyT8J+lVa69RkOj?=
 =?us-ascii?Q?SwdZMw=3D=3D?=
X-OriginatorOrg: corigine.com
X-MS-Exchange-CrossTenant-Network-Message-Id: d5398451-8e49-474d-92d9-08db20b787b1
X-MS-Exchange-CrossTenant-AuthSource: PH0PR13MB4842.namprd13.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Mar 2023 16:01:23.3066
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: fe128f2c-073b-4c20-818e-7246a585940c
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: GEz2bbRYaG7SE7fPw2nkOu/bvCenmCilow3y9Q7V1ujfkM3K9UzfrFe8ftAwiTkIYD+IYIySYnWdeuVY3lJEM3VzknwQbcPOFps6IA/1Em0=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR13MB5619
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Thu, Mar 09, 2023 at 03:56:36PM +0300, Daniil Tatianin wrote:
> Previously we would divide total_left_rate by zero if num_vports
> happened to be 1 because non_requested_count is calculated as
> num_vports - req_count. Guard against this by validating num_vports at
> the beginning and returning an error otherwise.

Thanks,

I like this approach.

Reviewed-by: Simon Horman <simon.horman@corigine.com>

> Found by Linux Verification Center (linuxtesting.org) with the SVACE
> static analysis tool.
> 
> Fixes: bcd197c81f63 ("qed: Add vport WFQ configuration APIs")
> Signed-off-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
> ---
>  drivers/net/ethernet/qlogic/qed/qed_dev.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/net/ethernet/qlogic/qed/qed_dev.c b/drivers/net/ethernet/qlogic/qed/qed_dev.c
> index d61cd32ec3b6..9aaaf5ad3eb0 100644
> --- a/drivers/net/ethernet/qlogic/qed/qed_dev.c
> +++ b/drivers/net/ethernet/qlogic/qed/qed_dev.c
> @@ -5083,6 +5083,13 @@ static int qed_init_wfq_param(struct qed_hwfn *p_hwfn,
>  
>  	num_vports = p_hwfn->qm_info.num_vports;
>  
> +	if (num_vports < 2) {
> +		DP_NOTICE(p_hwfn,
> +			   "Unexpected num_vports: %d\n",
> +			   num_vports);

nit: the lines above are not indented correctly, however,
     I think that it's better yet if the DP_NOTICE is on a single line.

> +		return -EINVAL;
> +	}
> +
>  	/* Accounting for the vports which are configured for WFQ explicitly */
>  	for (i = 0; i < num_vports; i++) {
>  		u32 tmp_speed;
> -- 
> 2.25.1
>