Re: [bug report] crypto: eip93 - Add Inside Secure SafeXcel EIP-93 crypto engine support

[Christian Marangi <ansuelsmth@gmail.com>]

On Wed, Feb 12, 2025 at 06:23:01PM +0300, Dan Carpenter wrote:
> Hello Christian Marangi,
> 
> Commit 9739f5f93b78 ("crypto: eip93 - Add Inside Secure SafeXcel
> EIP-93 crypto engine support") from Jan 14, 2025 (linux-next), leads
> to the following Smatch static checker warning:
> 
> drivers/crypto/inside-secure/eip93/eip93-common.c:233 check_valid_request() warn: 'src_nents' unsigned <= 0
> drivers/crypto/inside-secure/eip93/eip93-common.c:237 check_valid_request() warn: error code type promoted to positive: 'src_nents'
> drivers/crypto/inside-secure/eip93/eip93-common.c:240 check_valid_request() warn: error code type promoted to positive: 'dst_nents'
> 
> drivers/crypto/inside-secure/eip93/eip93-common.c
>     201 int check_valid_request(struct eip93_cipher_reqctx *rctx)
>     202 {
>     203         struct scatterlist *src = rctx->sg_src;
>     204         struct scatterlist *dst = rctx->sg_dst;
>     205         u32 src_nents, dst_nents;
>     206         u32 textsize = rctx->textsize;
>     207         u32 authsize = rctx->authsize;
>     208         u32 blksize = rctx->blksize;
>     209         u32 totlen_src = rctx->assoclen + rctx->textsize;
>     210         u32 totlen_dst = rctx->assoclen + rctx->textsize;
>     211         u32 copy_len;
>     212         bool src_align, dst_align;
>     213         int err = -EINVAL;
>     214 
>     215         if (!IS_CTR(rctx->flags)) {
>     216                 if (!IS_ALIGNED(textsize, blksize))
>     217                         return err;
>     218         }
>     219 
>     220         if (authsize) {
>     221                 if (IS_ENCRYPT(rctx->flags))
>     222                         totlen_dst += authsize;
>     223                 else
>     224                         totlen_src += authsize;
>     225         }
>     226 
>     227         src_nents = sg_nents_for_len(src, totlen_src);
>     228         dst_nents = sg_nents_for_len(dst, totlen_dst);
> 
> These return -EINVAL on error.
> 
>     229 
>     230         if (src == dst) {
>     231                 src_nents = max(src_nents, dst_nents);
>     232                 dst_nents = src_nents;
> --> 233                 if (unlikely((totlen_src || totlen_dst) && src_nents <= 0))
>                                                                    ^^^^^^^^^^^^^^
> It's unsigned so it can't be less than zero.
> 
>     234                         return err;
>     235 
>     236         } else {
>     237                 if (unlikely(totlen_src && src_nents <= 0))
>                                                    ^^^^^^^^^^^^^^
>     238                         return err;
>     239 
>     240                 if (unlikely(totlen_dst && dst_nents <= 0))
>                                                    ^^^^^^^^^^^^^^
> Same.
> 
>     241                         return err;
>     242         }
>     243 
>     244         if (authsize) {
>     245                 if (dst_nents == 1 && src_nents == 1) {
>     246                         src_align = eip93_is_sg_aligned(src, totlen_src, blksize);
>

Thanks, this wasn't reported in the first run so sorry for not noticing
this. I will take care of sending a follow-up patch to address this.

Again thanks for the report!

-- 
	Ansuel