issue wth ath12k in ath12k_dbring_buffer_release_event

issue wth ath12k in ath12k_dbring_buffer_release_event

[Colin King (gmail)]

[-- Attachment #1.1.1: Type: text/plain, Size: 2477 bytes --]

Hi,

In drivers/net/wireless/ath/ath12k/dbring.c function 
ath12k_dbring_buffer_release_event() there is a large hunk of code that 
is never executed because ring is NULL.  The code that is never executed 
is a fairly large part of the functionality of the code, so I this looks 
like a bug to me.

Analysis below, my comments prefixed by //

int ath12k_dbring_buffer_release_event(struct ath12k_base *ab,
                                  linux-kernel@vger.kernel.org 
struct ath12k_dbring_buf_release_event *ev)
{
         struct ath12k_dbring *ring = NULL;

// ring is set to NULL

         struct hal_srng *srng;
         struct ath12k *ar;
         struct ath12k_dbring_element *buff;
         struct ath12k_dbring_data handler_data;
         struct ath12k_buffer_addr desc;
         u8 *vaddr_unalign;
         u32 num_entry, num_buff_reaped;
         u8 pdev_idx, rbm;
         u32 cookie;
         int buf_id;
         int size;
         dma_addr_t paddr;
         int ret = 0;

         pdev_idx = le32_to_cpu(ev->fixed.pdev_id);

         if (pdev_idx >= ab->num_radios) {
                 ath12k_warn(ab, "Invalid pdev id %d\n", pdev_idx);
                 return -EINVAL;
         }

         if (ev->fixed.num_buf_release_entry !=
             ev->fixed.num_meta_data_entry) {
                 ath12k_warn(ab, "Buffer entry %d mismatch meta entry %d\n",
                             ev->fixed.num_buf_release_entry,
                             ev->fixed.num_meta_data_entry);
                 return -EINVAL;
         }

         ar = ab->pdevs[pdev_idx].ar;

         rcu_read_lock();
         if (!rcu_dereference(ab->pdevs_active[pdev_idx])) {
                 ret = -EINVAL;
                 goto rcu_unlock;
         }

         switch (ev->fixed.module_id) {
         case WMI_DIRECT_BUF_SPECTRAL:
                 break;
         default:
                 ring = NULL;
                 ath12k_warn(ab, "Recv dma buffer release ev on unsupp 
module %d\n",
                             ev->fixed.module_id);
                 break;
         }

// ring is still NULL

         if (!ring) {
                 ret = -EINVAL;
                 goto rcu_unlock;
         }

// the following code is *never* executed

         srng = &ab->hal.srng_list[ring->refill_srng.ring_id];
         num_entry = le32_to_cpu(ev->fixed.num_buf_release_entry);
         ....
         ....


Colin

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 4901 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: issue wth ath12k in ath12k_dbring_buffer_release_event

[Jeff Johnson]

On 5/8/2025 4:00 AM, Colin King (gmail) wrote:
> Hi,
> 
> In drivers/net/wireless/ath/ath12k/dbring.c function 
> ath12k_dbring_buffer_release_event() there is a large hunk of code that 
> is never executed because ring is NULL.  The code that is never executed 
> is a fairly large part of the functionality of the code, so I this looks 
> like a bug to me.

The features which use this functionality have not yet landed in the upstream
version of ath12k.

/jeff