Hi Dmitry,

On Thu, 2023-06-15 at 17:44 +0100, Dmitry Safonov wrote:

EXTERNAL EMAIL: Do not click links or open attachments unless you know the
content is safe

Hi Steen,

On 6/15/23 11:45, Steen Hegelund wrote:

quoted

Hi Dmitry,

On Thu, 2023-06-15 at 00:09 +0100, Dmitry Safonov wrote:

[..]

quoted

quoted

+/**
+ * tcp_sigpool_alloc_ahash - allocates pool for ahash requests
+ * @alg: name of async hash algorithm
+ * @scratch_size: reserve a tcp_sigpool::scratch buffer of this size
+ */
+int tcp_sigpool_alloc_ahash(const char *alg, size_t scratch_size)
+{
+       int i, ret;
+
+       /* slow-path */
+       mutex_lock(&cpool_mutex);
+       ret = sigpool_reserve_scratch(scratch_size);
+       if (ret)
+               goto out;
+       for (i = 0; i < cpool_populated; i++) {
+               if (!cpool[i].alg)
+                       continue;
+               if (strcmp(cpool[i].alg, alg))
+                       continue;
+
+               if (kref_read(&cpool[i].kref) > 0)
+                       kref_get(&cpool[i].kref);
+               else
+                       kref_init(&cpool[i].kref);
+               ret = i;
+               goto out;
+       }

Here it looks to me like you will never get to this part of the code since
you
always end up going to the out label in the previous loop.

Well, not exactly: this part is looking if the crypto algorithm is
already in this pool, so that it can increment refcounter rather than
initialize a new tfm. In case strcmp(cpool[i].alg, alg) fails, this loop
will never goto out.

Ah, right, you never find any algo and then get out at the end of the list.

I.e., you issued previously setsockopt()s for TCP-MD5 and TCP-AO with
HMAC-SHA1, so in this pool there'll be two algorithms: "md5" and
"hmac(sha1)". Now if you want to use TCP-AO with "cmac(aes128)" or
"hmac(sha256)", you won't find them in the pool yet.

quoted

quoted

+
+       for (i = 0; i < cpool_populated; i++) {
+               if (!cpool[i].alg)
+                       break;
+       }
+       if (i >= CPOOL_SIZE) {
+               ret = -ENOSPC;
+               goto out;
+       }
+
+       ret = __cpool_alloc_ahash(&cpool[i], alg);
+       if (!ret) {
+               ret = i;
+               if (i == cpool_populated)
+                       cpool_populated++;
+       }
+out:
+       mutex_unlock(&cpool_mutex);
+       return ret;
+}
+EXPORT_SYMBOL_GPL(tcp_sigpool_alloc_ahash);
+

... snip ...

quoted

 clear_hash:
-       tcp_put_md5sig_pool();
-clear_hash_noput:
+       tcp_sigpool_end(&hp);
+clear_hash_nostart:
        memset(md5_hash, 0, 16);
        return 1;
 }

Thanks,
            Dmitry

Reviewed-by: Steen Hegelund <Steen.Hegelund@microchip.com>

BR
Steen