From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 4C5FA285CA4 for ; Fri, 3 Jul 2026 16:42:57 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=170.10.133.124 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783096978; cv=none; b=imU46KD0dw9fbFP/EVgKUyRUw4cFUL28+9hAqcP+gtrSBh8IJhwVpqkljVooth0Ey5k6/mQ6q2aWCCGHrXeboQyar+yTJIqxUgaPkYgylhP2GoNL53ygcByXHIOlw/Tgh+DMelbTDwFYfrBEvu9a2hdJcAKYgQvgfNYoU3Zg2qs= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783096978; c=relaxed/simple; bh=LrO73FNsEp8JEgdsFcET9E6lQ82u0824/9f8DOWEEjY=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=oloIiYt2+IGMHyz/ib0tfftJjuwBoZCjJHm3vvTBx9sA26nRdbeOL/wnvIgvS8GmpS9EyZJG12Ca6ggYq7OzrJrKiW6DGmG0zHhlJoe/i88TF5Ff9OAYOyamFuYNKp5i5Pa1QG06dOYFg4VKtUE0SacZSwQTNXpVlg56r+7L91E= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com; spf=pass smtp.mailfrom=redhat.com; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b=E/Ol+7ZL; arc=none smtp.client-ip=170.10.133.124 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="E/Ol+7ZL" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1783096976; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=/RRtTWI8es+DNvxOnoS2/+a0IeV54OfACZzUtphWt54=; b=E/Ol+7ZLPEKS7aHP8AZUxr6sdQxlofDpkDvbqkAdBf6dETSh4KwWUtu8i0l3l0JVLsuJvD bz8IRDbKZbJ2xLU6oXsWHX+1sCYb0I38Tjetq1B/mnWAZPVEAtg4pr+usQ1a0jOvkFgni3 TRUJdfhOtOliZ6UMDK9jClNNt/4tkis= Received: from mx-prod-mc-06.mail-002.prod.us-west-2.aws.redhat.com (ec2-35-165-154-97.us-west-2.compute.amazonaws.com [35.165.154.97]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-688-nsZovw73Op2bqhGVfJ9nbA-1; Fri, 03 Jul 2026 12:42:53 -0400 X-MC-Unique: nsZovw73Op2bqhGVfJ9nbA-1 X-Mimecast-MFC-AGG-ID: nsZovw73Op2bqhGVfJ9nbA_1783096971 Received: from mx-prod-int-03.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-03.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.12]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-06.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 3EB9218908DC; Fri, 3 Jul 2026 16:42:51 +0000 (UTC) Received: from gerbillo.redhat.com (unknown [10.44.32.68]) by mx-prod-int-03.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id 689621944B04; Fri, 3 Jul 2026 16:42:46 +0000 (UTC) From: Paolo Abeni To: han.junyang@zte.com.cn Cc: andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com, kuba@kernel.org, pabeni@redhat.com, horms@kernel.org, linux-kernel@vger.kernel.org, netdev@vger.kernel.org, ran.ming@zte.com.cn, han.chengfei@zte.com.cn, zhang.yanze@zte.com.cn Subject: Re: [PATCH net-next v7 2/2] dinghai: add hardware register access and PCI capability scanning Date: Fri, 3 Jul 2026 18:42:43 +0200 Message-ID: <20260703164243.336173-1-pabeni@redhat.com> In-Reply-To: <20260630111532964R8a8-5zlzQRMas-VWfCi2@zte.com.cn> References: <20260630111532964R8a8-5zlzQRMas-VWfCi2@zte.com.cn> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Scanned-By: MIMEDefang 3.0 on 10.30.177.12 From: AI Reviewer This is an AI-generated review of your patch. The human sending this email has considered the AI review valid, or at least plausible. Full review at: https://sashiko.dev/ --- dinghai: add hardware register access and PCI capability scanning Implement PCI configuration space access, BAR mapping, capability scanning (common/notify/device), and hardware queue register definitions for DingHai PF device. > diff --git a/drivers/net/ethernet/zte/dinghai/en_pf.c b/drivers/net/ethernet/zte/dinghai/en_pf.c > index b492fd2c261b8..fb80b0baca34d 100644 > --- a/drivers/net/ethernet/zte/dinghai/en_pf.c > +++ b/drivers/net/ethernet/zte/dinghai/en_pf.c [ ... ] > @@ -104,6 +105,443 @@ void dh_pf_pci_close(struct dh_core_dev *dev) [ ... ] > +void __iomem *zxdh_pf_map_capability(struct dh_core_dev *dh_dev, int off, > + size_t minlen, u32 align, > + u32 start, u32 size, > + size_t *len, resource_size_t *pa, > + u32 *bar_off) > +{ [ ... ] > + if (length > size) > + length = size; > + > + if (len) > + *len = length; > + > + if (minlen + offset < minlen || > + minlen + offset > pci_resource_len(pdev, bar)) { [Severity: High] Does this code inflate the length returned in *len? Because length is not capped by pci_resource_len(pdev, bar) - offset before being assigned to *len, the returned length can be larger than the actually mapped memory (which is capped by pci_iomap_range). Later in zxdh_pf_map_vq_notify(), this inflated notify_len is used for bounds checking, which could allow out-of-bounds MMIO accesses to pass the check. [ ... ] > +int zxdh_pf_device_cfg_init(struct dh_core_dev *dh_dev) > +{ > + struct zxdh_pf_device *pf_dev = dh_dev->priv; > + struct pci_dev *pdev = dh_dev->pdev; > + int device; > + > + /* Device capability is only mandatory for > + * devices that have device-specific configuration. > + */ > + device = zxdh_pf_pci_find_capability(pdev, ZXDH_PCI_CAP_DEVICE_CFG, > + IORESOURCE_IO | IORESOURCE_MEM, > + &pf_dev->modern_bars); > + > + /* we don't know how much we should map, > + * but PAGE_SIZE is more than enough for all existing devices. > + */ > + if (device) { > + pf_dev->device = zxdh_pf_map_capability(dh_dev, device, 0, > + ZXDH_PF_ALIGN4, 0, PAGE_SIZE, > + &pf_dev->device_len, NULL, > + &pf_dev->dev_cfg_bar_off); > + if (!pf_dev->device) { [Severity: High] Could pf_dev->device remain NULL if the device capability isn't found? If the capability is optional and not provided, pf_dev->device will be NULL. Also, passing minlen = 0 allows undersized mappings. Later, zxdh_get_mac() and zxdh_set_mac() unconditionally dereference pf_dev->device, which could trigger a kernel panic due to a NULL pointer dereference or out-of-bounds access. [ ... ] > +void __iomem *zxdh_pf_map_vq_notify(struct dh_core_dev *dh_dev, > + u16 phy_index, u16 index, > + resource_size_t *pa) > +{ > + struct zxdh_pf_device *pf_dev = dh_dev->priv; > + u16 off; > + > + off = zxdh_pf_get_queue_notify_off(dh_dev, phy_index, index); > + > + if (pf_dev->notify_base) { [ ... ] > + } else { > + return zxdh_pf_map_capability(dh_dev, pf_dev->notify_map_cap, 2, 2, > + off * pf_dev->notify_offset_multiplier, > + 2, NULL, pa, NULL); > + } > +} [Severity: Medium] Can the offset calculation overflow before being passed? Since off is u16 and pf_dev->notify_offset_multiplier is u32, off * pf_dev->notify_offset_multiplier evaluates as a 32-bit unsigned integer. If the product exceeds 4GB, it wraps around before being passed to zxdh_pf_map_capability(). A 64-bit cast is needed to prevent mapping incorrect device registers. [ ... ] > +void zxdh_pf_get_vf_mac(struct dh_core_dev *dh_dev, u8 *mac, int vf_id) > +{ > + struct zxdh_pf_device *pf_dev = dh_dev->priv; > + u32 DEV_MAC_L; > + u16 DEV_MAC_H; > + > + if (pf_dev->pf_sriov_cap_base) { > + DEV_MAC_L = ioread32(pf_dev->pf_sriov_cap_base + > + (pf_dev->sriov_bar_size) * vf_id + > + pf_dev->dev_cfg_bar_off); > + mac[0] = DEV_MAC_L & 0xff; > + mac[1] = (DEV_MAC_L >> 8) & 0xff; > + mac[2] = (DEV_MAC_L >> 16) & 0xff; > + mac[3] = (DEV_MAC_L >> 24) & 0xff; > + DEV_MAC_H = ioread16(pf_dev->pf_sriov_cap_base + > + (pf_dev->sriov_bar_size) * vf_id + > + pf_dev->dev_cfg_bar_off + > + ZXDH_DEV_MAC_HIGH_OFFSET); > + mac[4] = DEV_MAC_H & 0xff; > + mac[5] = (DEV_MAC_H >> 8) & 0xff; > + } > +} [Severity: High] Does this code silently return without initializing the buffer? If pf_dev->pf_sriov_cap_base is NULL, the function exits without modifying the output mac buffer or returning an error code. The caller might proceed using the uninitialized contents of the array, which could expose uninitialized kernel stack bytes. [ ... ] > +void zxdh_set_mac(struct dh_core_dev *dh_dev, u8 *mac) > +{ > + struct zxdh_pf_device *pf_dev = dh_dev->priv; > + u32 DEV_MAC_L; > + u16 DEV_MAC_H; > + > + DEV_MAC_L = mac[0] | (mac[1] << 8) | (mac[2] << 16) | (mac[3] << 24); > + DEV_MAC_H = mac[4] | (mac[5] << 8); > + iowrite32(DEV_MAC_L, pf_dev->device); > + iowrite16(DEV_MAC_H, pf_dev->device + ZXDH_DEV_MAC_HIGH_OFFSET); > +} > + > +void zxdh_get_mac(struct dh_core_dev *dh_dev, u8 *mac) > +{ > + struct zxdh_pf_device *pf_dev = dh_dev->priv; > + u32 DEV_MAC_L; > + u16 DEV_MAC_H; > + > + DEV_MAC_L = ioread32(pf_dev->device); > + mac[0] = DEV_MAC_L & 0xff; > + mac[1] = (DEV_MAC_L >> 8) & 0xff; > + mac[2] = (DEV_MAC_L >> 16) & 0xff; > + mac[3] = (DEV_MAC_L >> 24) & 0xff; > + DEV_MAC_H = ioread16(pf_dev->device + ZXDH_DEV_MAC_HIGH_OFFSET); > + mac[4] = DEV_MAC_H & 0xff; > + mac[5] = (DEV_MAC_H >> 8) & 0xff; > +} [Severity: High] Can these functions trigger a NULL pointer dereference? In zxdh_pf_device_cfg_init(), pf_dev->device can be left as NULL if the device capability is missing, or it could be undersized. zxdh_set_mac() and zxdh_get_mac() unconditionally dereference pf_dev->device, which could trigger a kernel panic. -- This is an AI-generated review.