From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (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 C1CA44F0533 for ; Tue, 13 Oct 2026 11:46:07 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1791891969; cv=none; b=op2vG60DBSoSC7FvDvJHKx/VSZOFdtL94RTJJcSnL/9vFgrg8EXuzBqzUV/SpKT/e49bQ/84guSNwdbGY2oLJUiDikD3wG0oKw+E9yp/866T323uHn6la2F/fefrbwlRZWfLeHOkBa6plypbDhoDWUriGrTDMKahGbEP27OZkxE= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1791891969; c=relaxed/simple; bh=ZyJbO78R5SZget9WhlmXbUhHKbMSMYYMiFZLe6uk0ec=; h=Message-ID:From:Subject:To:Cc:In-Reply-To:References:Content-Type: Date; b=pfD47wa4NID0e2qq5RWy84UXOfcM2NPO74vFpTUbUZTKWTL+ZeK3YIXsABFvcHq3ONmRZwKIdM9uU+EMRKw32222i65bD8hCNFn7z5QTUnjF9mVjpbYvqj/rObO63jF0hCKe+Dl5NGc8zWk883PSy12LIRx1waUsYlYuvsZew8k= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=f+Ef9vLn; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="f+Ef9vLn" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 771D81F00893; Tue, 13 Oct 2026 11:46:07 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1791891967; bh=ZyJbO78R5SZget9WhlmXbUhHKbMSMYYMiFZLe6uk0ec=; h=From:Subject:Reply-To:To:Cc:In-Reply-To:References:Date; b=f+Ef9vLnqYq2cWAKsWdKSz1NCJC9uyYJ8gJs6uEah08gtGFmIsghuiauHhDSRY+/j dREoAtQdhHKdKNSg4lxq6EmPI1+lhDXST/CFf6OgSeEFVT5hhZ6uKaaSIjif2SK+cu F5OV1jPR+k96NU1ucUIqUKEs5Sw4VxF9VONqgaUcEknbYldg/xJNLwgxWorQ+vrsvT vXK5fDxy7SdfNsyofhzLbKNTcLcfdKONCle15aLNTpsGgWNTKh2wWCMm6khP0g2bUp QbvzdVVq6FLW6Ln3pTa0HLuC05be15hQvpwwAudJXFrUKhK0jL2iBAwhYgcV9zGbrq ph6OrQEpFgo7w== Message-ID: From: sashiko-bot@kernel.org Subject: Re: [PATCH 04/15] yunsilicon: add event queues, interrupts and the Reply-To: sashiko-reviews@lists.linux.dev To: "Xin Tian" Cc: linux-rdma@vger.kernel.org In-Reply-To: <20261012114359.2471317-5-tianx@yunsilicon.com> References: <20261012114352.2471317-1-tianx@yunsilicon.com> <20261012114359.2471317-5-tianx@yunsilicon.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Date: Tue, 13 Oct 2026 11:46:07 +0000 Precedence: bulk X-Mailing-List: linux-rdma@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Thank you for your contribution! Sashiko AI review found 8 potential issue(= s) to consider: - [High] net: xsc: bitwise XOR logic error in next_eqe_sw() halts event que= ue processing - [High] net: xsc: skipping teardown when firmware is offline causes Use-Af= ter-Free and resource leaks - [High] net: xsc: spoofing device->numa_node during DMA allocation causes = data races - [High] yunsilicon: vmap() on dma_alloc_coherent() memory creates illegal = cache aliases - [Medium] net: xsc: multiple memory leaks and incorrect affinity setup in = cpumask management - [Medium] net: xsc: vunmap skipped on 32-bit architectures causes vmalloc = memory leak - [Medium] net: xsc: data race on link_event_handler can lead to NULL point= er dereference - [Medium] net: xsc: xsc_core_vector2eqn leaves irqn uninitialized when MSI= -X is disabled -- Make it readable and polite without reproducing the patch diff context. [Severity: High] File: ..., Function: ..., Line: ... Your question here... Do not use '> diff' or any direct source recitation. Respond only with the = prose descriptions.commit a872f21fb08a814fac5b1a6b91049b643f0a1ed7 Author: Xin Tian yunsilicon: add event queues, interrupts and the auxiliary bus This commit adds event queues, MSI-X vector handling, and auxiliary bus glue for the yunsilicon driver. It sets up the PCI core to request vectors, create completion queues, and publish auxiliary devices for the ethernet and RDMA drivers. [Severity: High] In drivers/net/ethernet/yunsilicon/xsc/pci/eq.c, within the next_eqe_sw() function, the logic for advancing the consumer index performs a bitwise XOR against !!(eq->cons_index & eq->nent) with (eqe->owner_data & XSC_EQE_OWNER= ). Because XSC_EQE_OWNER is BIT(7) (128), when the hardware sets the owner bit, this evaluates to 128 ^ 1, which equals 129. Since 129 is non-zero, it retu= rns NULL instead of the event queue entry. Will this cause the event queue to h= alt completely after exactly one cycle? Would it be better to normalize the left side to a boolean first? [Severity: High] In drivers/net/ethernet/yunsilicon/xsc/pci/eq.c, within xsc_destroy_unmap_e= q(), and in drivers/net/ethernet/yunsilicon/xsc/pci/pci_irq.c within both xsc_free_irq_vectors() and xsc_free_irq(), the functions check if the firmw= are is available and return early if it is not. By returning early, critical cleanup operations like free_irq(), pci_free_irq_vectors(), and xsc_buf_free() are skipped. Does skipping free_irq() leave the interrupt handler active to dereference the subsequently freed `eq` object, causing a Use-After-Free? Will this also leak PCI MSI-X allocations? [Severity: Medium] In drivers/net/ethernet/yunsilicon/xsc/pci/pci_irq.c, the function set_comp_irq_affinity_hint() is called in a loop over the number of vectors. Inside this loop, it unconditionally allocates xdev->xps_cpumask using zalloc_cpumask_var(). Does this repeatedly overwrite the pointer and permanently leak memory on every iteration when CONFIG_CPUMASK_OFFSTACK=3Dy? Additionally, if the allocation fails, it returns without freeing eq->mask. Also, when setting the affinity hint, the code passes the newly allocated and empty eq->mask to irq_set_affinity_hint() instead of the populated xdev->xps_cpumask. Finally, if xsc_cmd_enable_msix() fails in xsc_irq_eq_create(), the error path jumps to err_free_irq_evnt, which skips clear_comp_irq_affinity_hints(). Will this leak all successfully allocated cpumasks? [Severity: Medium] In drivers/net/ethernet/yunsilicon/xsc/pci/alloc.c, within xsc_buf_free(), there is a check that only calls vunmap() if BITS_PER_LONG =3D=3D 64. Howev= er, in xsc_buf_alloc(), the memory is unconditionally mapped using vmap() when the requested size exceeds the direct mapping threshold. Will guarding the vunm= ap() with an architecture check permanently leak the mapping on 32-bit platforms every time a multi-page buffer is freed? [Severity: Medium] In drivers/net/ethernet/yunsilicon/xsc/pci/pci_irq.c, within xsc_event_work= (), the link event handler is checked for NULL before being called. However, xsc_irq_event_unregister() can concurrently set xdev->link_event_handler to NULL. Can the compiler issue two separate memory loads for the check and the call, leading to a NULL pointer dereference if the pointer is cleared betwe= en the two operations? Should this use READ_ONCE() to load the pointer into a local variable first? [Severity: High] In drivers/net/ethernet/yunsilicon/xsc/pci/alloc.c, the function xsc_dma_zalloc_coherent_node() calls set_dev_node() to temporarily modify the global device->numa_node field in order to force node-local DMA allocations. While this is protected locally by dev_res->alloc_mutex, other subsystems might call dev_to_node() concurrently. Can this data race cause other parallel threads to read the spoofed value and allocate memory on the wrong physical NUMA node? [Severity: Medium] In drivers/net/ethernet/yunsilicon/xsc/pci/pci_irq.c, the function xsc_core_vector2eqn() returns 0 (success) early if xdev->caps.msix_enable is false, without initializing the *irqn parameter. The caller, xsc_vector2eqn= (), passes this success along, which could lead to undefined behavior when it uses the uninitialized local variable. Should *irqn be initialized before returning on this path? [Severity: High] In drivers/net/ethernet/yunsilicon/xsc/pci/alloc.c, within xsc_buf_alloc(), memory allocated via dma_alloc_coherent() is converted to struct page point= ers using virt_to_page() or vmalloc_to_page(), and then remapped contiguously using vmap(..., PAGE_KERNEL). Because DMA coherent memory is not guaranteed to be backed by standard linear map pages, and applying PAGE_KERNEL to potentially uncached or write-combined memory creates conflicting cache aliases, is this architecturally illegal? Does this bypass DMA API safety constraints and risk memory corruption or kernel panics? --=20 Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20261012114352.2471= 317-1-tianx@yunsilicon.com?part=3D4