netdev - Re: [PATCH RESEND V2 net 6/7] net: hns3: fixed hclge_fetch_pf_reg accesses bar space out of bounds issue

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z2KV37WZL7cpPYKk@mev-dev.igk.intel.com>
Date: Wed, 18 Dec 2024 10:29:03 +0100
From: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
To: Jijie Shao <shaojijie@...wei.com>
Cc: davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
	pabeni@...hat.com, andrew+netdev@...n.ch, horms@...nel.org,
	shenjian15@...wei.com, wangpeiyang1@...wei.com,
	liuyonglong@...wei.com, chenhao418@...wei.com,
	jonathan.cameron@...wei.com, shameerali.kolothum.thodi@...wei.com,
	salil.mehta@...wei.com, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH RESEND V2 net 6/7] net: hns3: fixed hclge_fetch_pf_reg
 accesses bar space out of bounds issue

On Tue, Dec 17, 2024 at 09:08:38AM +0800, Jijie Shao wrote:
> From: Hao Lan <lanhao@...wei.com>
> 
> The TQP BAR space is divided into two segments. TQPs 0-1023 and TQPs
> 1024-1279 are in different BAR space addresses. However,
> hclge_fetch_pf_reg does not distinguish the tqp space information when
> reading the tqp space information. When the number of TQPs is greater
> than 1024, access bar space overwriting occurs.
> The problem of different segments has been considered during the
> initialization of tqp.io_base. Therefore, tqp.io_base is directly used
> when the queue is read in hclge_fetch_pf_reg.
> 
> The error message:
> 
> Unable to handle kernel paging request at virtual address ffff800037200000
> pc : hclge_fetch_pf_reg+0x138/0x250 [hclge]
> lr : hclge_get_regs+0x84/0x1d0 [hclge]
> Call trace:
>  hclge_fetch_pf_reg+0x138/0x250 [hclge]
>  hclge_get_regs+0x84/0x1d0 [hclge]
>  hns3_get_regs+0x2c/0x50 [hns3]
>  ethtool_get_regs+0xf4/0x270
>  dev_ethtool+0x674/0x8a0
>  dev_ioctl+0x270/0x36c
>  sock_do_ioctl+0x110/0x2a0
>  sock_ioctl+0x2ac/0x530
>  __arm64_sys_ioctl+0xa8/0x100
>  invoke_syscall+0x4c/0x124
>  el0_svc_common.constprop.0+0x140/0x15c
>  do_el0_svc+0x30/0xd0
>  el0_svc+0x1c/0x2c
>  el0_sync_handler+0xb0/0xb4
>  el0_sync+0x168/0x180
> 
> Fixes: 939ccd107ffc ("net: hns3: move dump regs function to a separate file")
> Signed-off-by: Hao Lan <lanhao@...wei.com>
> Signed-off-by: Jijie Shao <shaojijie@...wei.com>
> Signed-off-by: Paolo Abeni <pabeni@...hat.com>
> ---
>  drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_regs.c  | 9 +++++----
>  .../net/ethernet/hisilicon/hns3/hns3vf/hclgevf_regs.c    | 9 +++++----
>  2 files changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_regs.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_regs.c
> index 43c1c18fa81f..8c057192aae6 100644
> --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_regs.c
> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_regs.c
> @@ -510,9 +510,9 @@ static int hclge_get_dfx_reg(struct hclge_dev *hdev, void *data)
>  static int hclge_fetch_pf_reg(struct hclge_dev *hdev, void *data,
>  			      struct hnae3_knic_private_info *kinfo)
>  {
> -#define HCLGE_RING_REG_OFFSET		0x200
>  #define HCLGE_RING_INT_REG_OFFSET	0x4
>  
> +	struct hnae3_queue *tqp;
>  	int i, j, reg_num;
>  	int data_num_sum;
>  	u32 *reg = data;
> @@ -533,10 +533,11 @@ static int hclge_fetch_pf_reg(struct hclge_dev *hdev, void *data,
>  	reg_num = ARRAY_SIZE(ring_reg_addr_list);
>  	for (j = 0; j < kinfo->num_tqps; j++) {
You can define struct hnae3_queue *tqp here to limit the scope
(same in VF case).
>  		reg += hclge_reg_get_tlv(HCLGE_REG_TAG_RING, reg_num, reg);
> +		tqp = kinfo->tqp[j];
>  		for (i = 0; i < reg_num; i++)
> -			*reg++ = hclge_read_dev(&hdev->hw,
> -						ring_reg_addr_list[i] +
> -						HCLGE_RING_REG_OFFSET * j);
> +			*reg++ = readl_relaxed(tqp->io_base -
> +					       HCLGE_TQP_REG_OFFSET +
> +					       ring_reg_addr_list[i]);
>  	}
>  	data_num_sum += (reg_num + HCLGE_REG_TLV_SPACE) * kinfo->num_tqps;
>  
> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_regs.c b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_regs.c
> index 6db415d8b917..7d9d9dbc7560 100644
> --- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_regs.c
> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_regs.c
> @@ -123,10 +123,10 @@ int hclgevf_get_regs_len(struct hnae3_handle *handle)
>  void hclgevf_get_regs(struct hnae3_handle *handle, u32 *version,
>  		      void *data)
>  {
> -#define HCLGEVF_RING_REG_OFFSET		0x200
>  #define HCLGEVF_RING_INT_REG_OFFSET	0x4
>  
>  	struct hclgevf_dev *hdev = hclgevf_ae_get_hdev(handle);
> +	struct hnae3_queue *tqp;
>  	int i, j, reg_um;
>  	u32 *reg = data;
>  
> @@ -147,10 +147,11 @@ void hclgevf_get_regs(struct hnae3_handle *handle, u32 *version,
>  	reg_um = ARRAY_SIZE(ring_reg_addr_list);
>  	for (j = 0; j < hdev->num_tqps; j++) {
>  		reg += hclgevf_reg_get_tlv(HCLGEVF_REG_TAG_RING, reg_um, reg);
> +		tqp = &hdev->htqp[j].q;
>  		for (i = 0; i < reg_um; i++)
> -			*reg++ = hclgevf_read_dev(&hdev->hw,
> -						  ring_reg_addr_list[i] +
> -						  HCLGEVF_RING_REG_OFFSET * j);
> +			*reg++ = readl_relaxed(tqp->io_base -
> +					       HCLGEVF_TQP_REG_OFFSET +
> +					       ring_reg_addr_list[i]);
>  	}
>  
>  	reg_um = ARRAY_SIZE(tqp_intr_reg_addr_list);
> -- 
> 2.33.0