netdev - 回复：[v4,net-next,11/11] net/nebula-matrix: add common dev start/stop operation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <8620f8c9-c7ff-427d-bad4-0ed860b998ea.Illusion.Wang@nebula-matrix.com>
Date: Tue, 10 Feb 2026 10:07:01 +0800
From: "Illusion Wang" <Illusion.Wang@...ula-matrix.com>
To: "Jakub Kicinski" <kuba@...nel.org>
Cc: "Sam" <sam.chen@...ula-matrix.com>,
  "lukas.bulwahn" <lukas.bulwahn@...hat.com>,
  "Dimon" <dimon.zhao@...ula-matrix.com>,
  "netdev" <netdev@...r.kernel.org>,
  "vadim.fedorenko" <vadim.fedorenko@...ux.dev>,
  "open list" <linux-kernel@...r.kernel.org>,
  "corbet" <corbet@....net>,
  "lorenzo" <lorenzo@...nel.org>,
  "Alvin" <alvin.wang@...ula-matrix.com>,
  "andrew+netdev" <andrew+netdev@...n.ch>,
  "linux-doc" <linux-doc@...r.kernel.org>,
  "pabeni" <pabeni@...hat.com>,
  "edumazet" <edumazet@...gle.com>,
  "horms" <horms@...nel.org>
Subject: 回复：[v4,net-next,11/11] net/nebula-matrix: add common dev start/stop operation

Thank you for your feedback
But
enum nbl_msix_serv_type {
	/* virtio_dev has a config vector_id, and the vector_id need is 0 */
	NBL_MSIX_VIRTIO_TYPE = 0,
	NBL_MSIX_NET_TYPE,
	NBL_MSIX_MAILBOX_TYPE,
	NBL_MSIX_TYPE_MAX
};
NBL_MSIX_NET_TYPE equals 1，so this function has no problem？

--illusion.wang

> +static int nbl_dev_configure_msix_map(struct nbl_dev_mgt *dev_mgt)
> +{
> + struct nbl_dispatch_ops *disp_ops = dev_mgt->disp_ops_tbl->ops;
> + struct nbl_dev_common *dev_common = dev_mgt->common_dev;
> + struct nbl_msix_info *msix_info = &dev_common->msix_info;
> + bool mask_en = msix_info->serv_info[NBL_MSIX_NET_TYPE].hw_self_mask_en;
> + u16 msix_net_num = msix_info->serv_info[NBL_MSIX_NET_TYPE].num;
> + u16 msix_not_net_num = 0;
> + int err, i;
> +
> + for (i = NBL_MSIX_NET_TYPE; i < NBL_MSIX_TYPE_MAX; i++)
> +  msix_info->serv_info[i].base_vector_id =
> +   msix_info->serv_info[i - 1].base_vector_id +
> +   msix_info->serv_info[i - 1].num;
             ^^^^^^

Does this loop cause an array underflow? NBL_MSIX_NET_TYPE equals 0 (from
the enum in nbl_dev.h), so on the first iteration i=0 and the code accesses
serv_info[-1].

The serv_info array is declared with NBL_MSIX_TYPE_MAX (2) elements in
struct nbl_msix_info, giving valid indices [0,1]. Accessing index -1 reads
uninitialized memory before the array start.

Call chain: nbl_core_start()->nbl_dev_start()->nbl_dev_start_common_dev()
->nbl_dev_configure_msix_map() guarantees this occurs during device start.

Should the loop start at i=1 (NBL_MSIX_MAILBOX_TYPE) instead, or should
serv_info[0].base_vector_id be initialized before the loop?

> +
> + for (i = NBL_MSIX_MAILBOX_TYPE; i < NBL_MSIX_TYPE_MAX; i++) {

[ ... ]

Thank you for pointing this out! Upon reviewing the code again, I believe
the variable declarations in nbl_dev_request_mailbox_irq() already follow
the reverse Christmas tree convention:

dev_common (longest identifier) is declared first, as it is required for
subsequent declarations.

The remaining variables (msix_info and common) are ordered by decreasing 
dentifier length, aligning with the style guide.
However, I appreciate the clarification opportunity—if there’s a nuance
I’m missing (e.g., stricter adherence to identifier length over dependency
order), I’d be happy to adjust the code accordingly.creasing identifier
length, aligning with the style guide.

---illusion.wang

Andrew Lunn also noted in v3 that variable declarations should follow
reverse Christmas tree ordering (longest line first). Looking at functions
in this commit like nbl_dev_request_mailbox_irq():

 struct nbl_dev_common *dev_common = dev_mgt->common_dev;
 struct nbl_msix_info *msix_info = &dev_common->msix_info;
 struct nbl_common_info *common = dev_mgt->common;

the declarations are not in reverse Christmas tree order.

Link: https://lore.kernel.org/netdev/c6d37ecd-7dc6-4f83-b76c-2735a5f116fd@lunn.ch/