Hi Dan,
On 10/31/23 09:51, Dan Carpenter wrote:
> The "cpool_populated" variable is the number of elements in the cpool[]
> array that have been populated. It is incremented in
> tcp_sigpool_alloc_ahash() every time we populate a new element.
> Unpopulated elements are NULL but if we have populated every element then
> this code will read one element beyond the end of the array.
>
> Fixes: 8c73b26315aa ("net/tcp: Prepare tcp_md5sig_pool for TCP-AO")
> Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
Yeah, those are barriers for any issues in the caller-code, so that's
not too critical but nice to have. Thanks for the patch!
Reviewed-by: Dmitry Safonov <dima@xxxxxxxxxx>
> ---
> From static analysis and review.
>
> net/ipv4/tcp_sigpool.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/net/ipv4/tcp_sigpool.c b/net/ipv4/tcp_sigpool.c
> index 65a8eaae2fec..55b310a722c7 100644
> --- a/net/ipv4/tcp_sigpool.c
> +++ b/net/ipv4/tcp_sigpool.c
> @@ -231,7 +231,7 @@ static void cpool_schedule_cleanup(struct kref *kref)
> */
> void tcp_sigpool_release(unsigned int id)
> {
> - if (WARN_ON_ONCE(id > cpool_populated || !cpool[id].alg))
> + if (WARN_ON_ONCE(id >= cpool_populated || !cpool[id].alg))
> return;
>
> /* slow-path */
> @@ -245,7 +245,7 @@ EXPORT_SYMBOL_GPL(tcp_sigpool_release);
> */
> void tcp_sigpool_get(unsigned int id)
> {
> - if (WARN_ON_ONCE(id > cpool_populated || !cpool[id].alg))
> + if (WARN_ON_ONCE(id >= cpool_populated || !cpool[id].alg))
> return;
> kref_get(&cpool[id].kref);
> }
> @@ -256,7 +256,7 @@ int tcp_sigpool_start(unsigned int id, struct tcp_sigpool *c) __cond_acquires(RC
> struct crypto_ahash *hash;
>
> rcu_read_lock_bh();
> - if (WARN_ON_ONCE(id > cpool_populated || !cpool[id].alg)) {
> + if (WARN_ON_ONCE(id >= cpool_populated || !cpool[id].alg)) {
> rcu_read_unlock_bh();
> return -EINVAL;
> }
> @@ -301,7 +301,7 @@ EXPORT_SYMBOL_GPL(tcp_sigpool_end);
> */
> size_t tcp_sigpool_algo(unsigned int id, char *buf, size_t buf_len)
> {
> - if (WARN_ON_ONCE(id > cpool_populated || !cpool[id].alg))
> + if (WARN_ON_ONCE(id >= cpool_populated || !cpool[id].alg))
> return -EINVAL;
>
> return strscpy(buf, cpool[id].alg, buf_len);
Thanks,
Dmitry
