Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 27 May 2022 11:23:52 +0300
From:   Dan Carpenter <dan.carpenter@oracle.com>
To:     Shijith Thotton <sthotton@marvell.com>
Cc:     Arnaud Ebalard <arno@natisbad.org>,
        Herbert Xu <herbert@gondor.apana.org.au>,
        Boris Brezillon <bbrezillon@kernel.org>,
        linux-crypto@vger.kernel.org, jerinj@marvell.com,
        sgoutham@marvell.com, Srujana Challa <schalla@marvell.com>,
        "David S. Miller" <davem@davemloft.net>,
        Harman Kalra <hkalra@marvell.com>,
        Yang Yingliang <yangyingliang@huawei.com>,
        Kees Cook <keescook@chromium.org>,
        Jiapeng Chong <jiapeng.chong@linux.alibaba.com>,
        open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] crypto: octeontx2: fix potential null pointer access
Message-ID: <20220527082352.GP2168@kadam>
References: <da89ba20819ad3ca6b99f9ef056f2bc1b076dc6d.1653632699.git.sthotton@marvell.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <da89ba20819ad3ca6b99f9ef056f2bc1b076dc6d.1653632699.git.sthotton@marvell.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?3GNvNDu/6rfd17USS/E99O6kKoQNEslnq47DHSiR4Uux8oxTo8+k78lsa9+l?=
 =?us-ascii?Q?SVxP/ATxfz5HBAGX9WK+gvlOKUE4/nvZJX4MOyGphDI9XjvnJmLbqzWe4rlx?=
 =?us-ascii?Q?bC0QvjGyWTomgJRLyvGCRL+/W8Fe+KOTkWJdCjHoZNXFseIdclPj+iKXZU20?=
 =?us-ascii?Q?of7zhMd6wOi0KVxHUxzgNarowlfcpHgM8qTm5Kr4y5G0ikDg4XXJwodNbx3x?=
 =?us-ascii?Q?kmJQPKpVUrnLwQpp1SdskCFiHLhizRrGbFLNe9B966EimmYn2VLYhyjOGsfr?=
 =?us-ascii?Q?wMcg0tTFTlkhbOXf9WvF4LSg3jroTJPo7zotsWYSxUiktYpyOK5z0Sns5CzD?=
 =?us-ascii?Q?j5rEVv81SFcI/FRdR4PObv+CwBSmbk2YIK4bAg8awESfLH4e1zDhx6Q8V7FV?=
 =?us-ascii?Q?krk5lnXKp9/dnYF4nid3+Vf52KUz71Fe7uzVGygOh3jJeEfR0/nkIv0JgCUn?=
 =?us-ascii?Q?B62jd9ccYoAyyYCUkK74vUuUWDbmaIjq0Ro8Is+pSWxjhw3xYNbTWPQst6cS?=
 =?us-ascii?Q?r05L+zUeAUnQJoIxkPXHXV2M1Z6LoTvlXOgfrCWsuVM3q/UCZDjj2465c8qv?=
 =?us-ascii?Q?tv4RrYfZluji1uImfZPcNQhEi8rvjJba8f32uyqd/UQTQ0X5UaN+fHrKoNgd?=
 =?us-ascii?Q?eTrEvXSE+1BCG2cRYKy2BYi1yWEQftucXQaCmm+b5Xo48WNRkSPzYfpTVKrR?=
 =?us-ascii?Q?OCgKuQUwJixla9Gs494lr7MJFDTM5HPc9U05GoXtSK90BHuaHCuFUC3MZidJ?=
 =?us-ascii?Q?ECkcmlZ6FRqs0hbYuqV1SbQPceSCOEQLLg3Uq0i07Bn7s3B+76R9n5o9kOmq?=
 =?us-ascii?Q?juuE+kRbBCLbPSiK8JYc8344StgD5lmAFzfIjEV0d7lLYhCexdgDKihuyv8U?=
 =?us-ascii?Q?aXtoMSJdgGVcXzXBRkL8YWCdviK8CX46LXh3K/tz70EV8FzvhRQpEQVEK0vk?=
 =?us-ascii?Q?LdvAVy3V1P/5cVpusfWBe3Zrro+Nbdrmo+w3wtzkz8iEZ7pmcn9XexT1ViJW?=
 =?us-ascii?Q?cgL6FfA0OB8BvMJt0alfnB/uy738aHJiVflJeQr12fnTSDQeDWwTQnEFklDM?=
 =?us-ascii?Q?H9IBOkRo5kUkgoZizrOgSmRw7Zql/NXEdBch4ODwpe3Xe/2a7mhOB8FG3CDf?=
 =?us-ascii?Q?nAHQNx3k9sX/VgZQEkv7fRie45u9W9ts/J2Qm/5BTqrDDCZLn1jDu9fLR6qW?=
 =?us-ascii?Q?heP8oc+n9fOVVV026R0cPogT7jQciHCGPTtw+DzI8QRSg0j5Q8aueuP6c7+a?=
 =?us-ascii?Q?ZcHNxUe1DwBkMLXVnytrQhap/DjHtDs+RluNPeP+P1o7XNqPQTVecAc/H4vR?=
 =?us-ascii?Q?PkFXJV993jO7QXBG0mVERjTACMJ4ZbH/hoh8TW+SnEuFi/wgnrOPm6pLKG0q?=
 =?us-ascii?Q?MMhJIyr7ag69loxxFUNg4rqjSbEsGsEWzyOy17vxRM3dxYLWUfmS0QUa6fa1?=
 =?us-ascii?Q?Wsvpp7Zu+G2GhzllqK1i+2XdtOKKMtGAr0MZRj3owkfpDOSEPozY+IBxaISd?=
 =?us-ascii?Q?TyU2owpwavGdrx3stbX0QFTg+ipXFWI3kf0KfbgMmU+KVrYgEjb30u45bXdC?=
 =?us-ascii?Q?pnXst+h6Ts4B0WU2nWUILcUZ+5GVAImjJIicG+oY0pG7KUUPx9kKp3/20iYp?=
 =?us-ascii?Q?ogvllHhB8VicqN84+yudhAsDMbgIQdFa93HCiYlnpX841e2Gq5/RGyGNhCzt?=
 =?us-ascii?Q?ChB086F88gj/E+Q/XqorGH40kNuwmw9sPzOS8i9kmm48Yr6WquC5d+CLsNdU?=
 =?us-ascii?Q?O5YgrYK/nRaTKqghlZlqLJn0EKXu1CA=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 299c5059-7bd6-47fe-74cd-08da3fba4995
X-MS-Exchange-CrossTenant-AuthSource: MWHPR1001MB2365.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 27 May 2022 08:24:15.8438
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: Rr8INVbquk08DkzMWyhVdiLgD884iUD+/fQFIdLwZEmU1XAKY5TjK+9Xt6Sv6lw61rHTStKVRxOgmECfWAg39WCwpho5bE06R5I97bqdgY8=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM8PR10MB5462
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.486,18.0.874
 definitions=2022-05-27_02:2022-05-25,2022-05-27 signatures=0
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxlogscore=999 malwarescore=0
 adultscore=0 bulkscore=0 suspectscore=0 phishscore=0 mlxscore=0
 spamscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2204290000 definitions=main-2205270039
X-Proofpoint-GUID: oRMevPDBTkpvcHs7L51x9uHoAiMZ-L-N
X-Proofpoint-ORIG-GUID: oRMevPDBTkpvcHs7L51x9uHoAiMZ-L-N
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE
        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-crypto.vger.kernel.org>
X-Mailing-List: linux-crypto@vger.kernel.org

On Fri, May 27, 2022 at 01:27:56PM +0530, Shijith Thotton wrote:
> diff --git a/drivers/crypto/marvell/octeontx2/otx2_cptpf_ucode.c b/drivers/crypto/marvell/octeontx2/otx2_cptpf_ucode.c
> index 9cba2f714c7e..b91401929fc6 100644
> --- a/drivers/crypto/marvell/octeontx2/otx2_cptpf_ucode.c
> +++ b/drivers/crypto/marvell/octeontx2/otx2_cptpf_ucode.c
> @@ -1605,7 +1605,11 @@ int otx2_cpt_dl_custom_egrp_create(struct otx2_cptpf_dev *cptpf,
>  		if (!strncasecmp(val, "se", 2) && strchr(val, ':')) {
>  			if (has_se || ucode_idx)
>  				goto err_print;
> -			tmp = strim(strsep(&val, ":"));
> +			tmp = strsep(&val, ":");
> +			if (tmp != NULL)
> +				tmp = strim(tmp);
> +			else
> +				goto err_print;

The check is not needed here, but if it were then the better way to
write this would be:

		tmp = strsep(&val, ":");
		if (!tmp)
			goto err_print;
		tmp = strim(tmp);

Always to error handling, not success handling.  checkpatch.pl --strict
will complain about the != NULL.

regards,
dan carpenter