* [PATCH net-next 1/2] seg6: add FIB table attribute for post-encap SID route lookup
From: Andrea Mayer @ 2026-07-11 16:29 UTC (permalink / raw)
To: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: David Ahern, Simon Horman, Shuah Khan, Nicolas Dichtel,
Justin Iurman, Anthony Doeraene, Stefano Salsano,
Ahmed Abdelsalam, Paolo Lungaroni, netdev, linux-kselftest,
linux-kernel, Andrea Mayer
In-Reply-To: <20260711162907.6521-1-andrea.mayer@uniroma2.it>
After SRv6 encapsulation the kernel looks up the route for the first SID,
that is the outer IPv6 destination of the encapsulated packet. This
post-encap SID route lookup uses the FIB table of the current routing
context. When the encap route is installed in a VRF, the VRF's table may
not have a route matching the SID. In that case another table should
handle it, e.g. one configured for underlay connectivity.
Add an optional SEG6_IPTUNNEL_TABLE attribute that selects the FIB table
used for this lookup. When set by the user, the attribute is honored on
both the input path (forwarded traffic) and the output path (locally
originated traffic). SRv6 encap routes that do not set the attribute use
the current routing context, as before.
For example:
# SID route installed in the underlay table 500
ip -6 route add fc00::100/128 via fd00::1 dev veth0 table 500
# encap route in vrf-100; the first SID is looked up in table 500
ip -6 route add cafe::1/128 vrf vrf-100 \
encap seg6 mode encap segs fc00::100 lookup 500 dev veth0
# or look up the SID in the main table
ip -6 route add cafe::1/128 vrf vrf-100 \
encap seg6 mode encap segs fc00::100 lookup main dev veth0
Suggested-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Signed-off-by: Andrea Mayer <andrea.mayer@uniroma2.it>
Reviewed-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
include/uapi/linux/seg6_iptunnel.h | 1 +
net/ipv6/seg6_iptunnel.c | 132 +++++++++++++++++++++++++----
2 files changed, 117 insertions(+), 16 deletions(-)
diff --git a/include/uapi/linux/seg6_iptunnel.h b/include/uapi/linux/seg6_iptunnel.h
index 485889b19900..e1964b3a4bb0 100644
--- a/include/uapi/linux/seg6_iptunnel.h
+++ b/include/uapi/linux/seg6_iptunnel.h
@@ -21,6 +21,7 @@ enum {
SEG6_IPTUNNEL_UNSPEC,
SEG6_IPTUNNEL_SRH,
SEG6_IPTUNNEL_SRC, /* struct in6_addr */
+ SEG6_IPTUNNEL_TABLE, /* __u32 FIB table for post-encap SID lookup */
__SEG6_IPTUNNEL_MAX,
};
#define SEG6_IPTUNNEL_MAX (__SEG6_IPTUNNEL_MAX - 1)
diff --git a/net/ipv6/seg6_iptunnel.c b/net/ipv6/seg6_iptunnel.c
index 4c45c0a77d75..61c6a27bf202 100644
--- a/net/ipv6/seg6_iptunnel.c
+++ b/net/ipv6/seg6_iptunnel.c
@@ -51,6 +51,7 @@ struct seg6_lwt {
struct dst_cache cache_input;
struct dst_cache cache_output;
struct in6_addr tunsrc;
+ u32 table;
struct seg6_iptunnel_encap tuninfo[];
};
@@ -68,6 +69,7 @@ seg6_encap_lwtunnel(struct lwtunnel_state *lwt)
static const struct nla_policy seg6_iptunnel_policy[SEG6_IPTUNNEL_MAX + 1] = {
[SEG6_IPTUNNEL_SRH] = { .type = NLA_BINARY },
[SEG6_IPTUNNEL_SRC] = NLA_POLICY_EXACT_LEN(sizeof(struct in6_addr)),
+ [SEG6_IPTUNNEL_TABLE] = { .type = NLA_U32 },
};
static int nla_put_srh(struct sk_buff *skb, int attrtype,
@@ -479,6 +481,73 @@ int seg6_do_srh_inline(struct sk_buff *skb, struct ipv6_sr_hdr *osrh)
}
EXPORT_SYMBOL_GPL(seg6_do_srh_inline);
+/* look up a route in a specific FIB table.
+ * Returns a refcounted dst, or NULL if the table does not exist.
+ */
+static struct dst_entry *seg6_table_lookup(struct net *net,
+ struct sk_buff *skb,
+ struct flowi6 *fl6, u32 tbl_id)
+{
+ struct fib6_table *table;
+ struct rt6_info *rt;
+
+ table = fib6_get_table(net, tbl_id);
+ if (!table)
+ return NULL;
+
+ rt = ip6_pol_route(net, table, 0, fl6, skb, RT6_LOOKUP_F_HAS_SADDR);
+ return &rt->dst;
+}
+
+static void seg6_init_flowi6(struct sk_buff *skb, struct ipv6hdr *hdr,
+ struct flowi6 *fl6)
+{
+ memset(fl6, 0, sizeof(*fl6));
+
+ fl6->daddr = hdr->daddr;
+ fl6->saddr = hdr->saddr;
+ fl6->flowlabel = ip6_flowinfo(hdr);
+ fl6->flowi6_mark = skb->mark;
+ fl6->flowi6_proto = hdr->nexthdr;
+}
+
+/* look up the route for the first SID on the input path and set it on the skb.
+ * Returns the refcounted dst, or NULL if a reference could not be safely taken.
+ */
+static struct dst_entry *seg6_input_route(struct net *net,
+ struct sk_buff *skb,
+ struct seg6_lwt *slwt)
+{
+ u32 table = slwt->table;
+
+ if (table) {
+ struct ipv6hdr *hdr = ipv6_hdr(skb);
+ struct dst_entry *dst;
+ struct flowi6 fl6;
+
+ seg6_init_flowi6(skb, hdr, &fl6);
+ fl6.flowi6_iif = skb->dev->ifindex;
+
+ dst = seg6_table_lookup(net, skb, &fl6, table);
+ if (!dst) {
+ dst = &net->ipv6.ip6_blk_hole_entry->dst;
+ dst_hold(dst);
+ }
+
+ skb_dst_drop(skb);
+ skb_dst_set(skb, dst);
+ } else {
+ ip6_route_input(skb);
+
+ /* ip6_route_input() sets a NOREF dst; force a refcount on it
+ * before caching or further use.
+ */
+ skb_dst_force(skb);
+ }
+
+ return skb_dst(skb);
+}
+
static int seg6_input_finish(struct net *net, struct sock *sk,
struct sk_buff *skb)
{
@@ -513,15 +582,9 @@ static int seg6_input_core(struct net *net, struct sock *sk,
goto drop;
}
- if (!dst) {
- ip6_route_input(skb);
-
- /* ip6_route_input() sets a NOREF dst; force a refcount on it
- * before caching or further use.
- */
- skb_dst_force(skb);
- dst = skb_dst(skb);
- if (unlikely(!dst)) {
+ if (unlikely(!dst)) {
+ dst = seg6_input_route(net, skb, slwt);
+ if (!dst) {
err = -ENETUNREACH;
goto drop;
}
@@ -578,6 +641,29 @@ static int seg6_input(struct sk_buff *skb)
return seg6_input_core(dev_net(skb->dev), NULL, skb);
}
+/* look up the route for the first SID on the output path. Always returns a
+ * refcounted dst.
+ */
+static struct dst_entry *seg6_output_dst_lookup(struct net *net,
+ struct sk_buff *skb,
+ struct flowi6 *fl6,
+ struct seg6_lwt *slwt)
+{
+ struct dst_entry *dst;
+
+ if (slwt->table) {
+ dst = seg6_table_lookup(net, skb, fl6, slwt->table);
+ if (!dst) {
+ dst = &net->ipv6.ip6_blk_hole_entry->dst;
+ dst_hold(dst);
+ }
+ } else {
+ dst = ip6_route_output(net, NULL, fl6);
+ }
+
+ return dst;
+}
+
static int seg6_output_core(struct net *net, struct sock *sk,
struct sk_buff *skb)
{
@@ -600,14 +686,9 @@ static int seg6_output_core(struct net *net, struct sock *sk,
struct ipv6hdr *hdr = ipv6_hdr(skb);
struct flowi6 fl6;
- memset(&fl6, 0, sizeof(fl6));
- fl6.daddr = hdr->daddr;
- fl6.saddr = hdr->saddr;
- fl6.flowlabel = ip6_flowinfo(hdr);
- fl6.flowi6_mark = skb->mark;
- fl6.flowi6_proto = hdr->nexthdr;
+ seg6_init_flowi6(skb, hdr, &fl6);
- dst = ip6_route_output(net, NULL, &fl6);
+ dst = seg6_output_dst_lookup(net, skb, &fl6, slwt);
if (dst->error) {
err = dst->error;
goto drop;
@@ -752,6 +833,15 @@ static int seg6_build_state(struct net *net, struct nlattr *nla,
}
}
+ if (tb[SEG6_IPTUNNEL_TABLE]) {
+ slwt->table = nla_get_u32(tb[SEG6_IPTUNNEL_TABLE]);
+ if (!slwt->table) {
+ NL_SET_ERR_MSG(extack, "invalid lookup table");
+ err = -EINVAL;
+ goto err_destroy_output;
+ }
+ }
+
newts->type = LWTUNNEL_ENCAP_SEG6;
newts->flags |= LWTUNNEL_STATE_INPUT_REDIRECT;
@@ -795,6 +885,10 @@ static int seg6_fill_encap_info(struct sk_buff *skb,
nla_put_in6_addr(skb, SEG6_IPTUNNEL_SRC, &slwt->tunsrc))
return -EMSGSIZE;
+ if (slwt->table &&
+ nla_put_u32(skb, SEG6_IPTUNNEL_TABLE, slwt->table))
+ return -EMSGSIZE;
+
return 0;
}
@@ -809,6 +903,9 @@ static int seg6_encap_nlsize(struct lwtunnel_state *lwtstate)
if (!ipv6_addr_any(&slwt->tunsrc))
nlsize += nla_total_size(sizeof(slwt->tunsrc));
+ if (slwt->table)
+ nlsize += nla_total_size(sizeof(u32));
+
return nlsize;
}
@@ -826,6 +923,9 @@ static int seg6_encap_cmp(struct lwtunnel_state *a, struct lwtunnel_state *b)
if (!ipv6_addr_equal(&a_slwt->tunsrc, &b_slwt->tunsrc))
return 1;
+ if (a_slwt->table != b_slwt->table)
+ return 1;
+
return memcmp(a_hdr, b_hdr, len);
}
--
2.20.1
^ permalink raw reply related
* [PATCH net-next 0/2] seg6: add FIB table attribute for post-encap SID route lookup
From: Andrea Mayer @ 2026-07-11 16:29 UTC (permalink / raw)
To: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: David Ahern, Simon Horman, Shuah Khan, Nicolas Dichtel,
Justin Iurman, Anthony Doeraene, Stefano Salsano,
Ahmed Abdelsalam, Paolo Lungaroni, netdev, linux-kselftest,
linux-kernel, Andrea Mayer
After SRv6 encapsulation the kernel looks up the route for the first SID,
the outer IPv6 destination of the encapsulated packet. This post-encap SID
route lookup uses the FIB table of the current routing context. When the
encap route is installed in a VRF, the VRF's table may not have a route for
the SID, which should be handled by another table, e.g. one used for
underlay connectivity.
A new optional SEG6_IPTUNNEL_TABLE attribute selects the FIB table used for
this lookup. When set by the user, the attribute is honored on both the
input path (traffic that is received, encapsulated and forwarded) and the
output path (traffic that is locally originated and then encapsulated).
SRv6 encap routes that do not set the attribute use the current routing
context, as before.
A companion iproute2 series follows on the mailing list. The examples below
show how to use the "lookup" attribute:
# SID route installed in the underlay table 500
ip -6 route add fc00::100/128 via fd00::1 dev veth0 table 500
# encap route in vrf-100; the first SID is looked up in table 500
ip -6 route add cafe::1/128 vrf vrf-100 \
encap seg6 mode encap segs fc00::100 lookup 500 dev veth0
# or if the SID is already handled by the main table
ip -6 route add cafe::1/128 vrf vrf-100 \
encap seg6 mode encap segs fc00::100 lookup main dev veth0
This work started from a use case raised by Nicolas Dichtel and took shape
in the discussion with him [1]. Thanks Nicolas.
The series is made of two patches. The first implements the attribute. The
second adds an L3 VPN selftest that exercises both the input and the output
path, with the attribute (traffic reaches its destination) and without it
(the packet is dropped).
Thanks,
Andrea
[1] https://lore.kernel.org/all/20260327140709.959636-1-nicolas.dichtel@6wind.com/T/
Andrea Mayer (2):
seg6: add FIB table attribute for post-encap SID route lookup
selftests: seg6: add test for post-encap SID route lookup
include/uapi/linux/seg6_iptunnel.h | 1 +
net/ipv6/seg6_iptunnel.c | 132 ++-
tools/testing/selftests/net/Makefile | 1 +
.../net/srv6_encap_lookup_l3vpn_test.sh | 1027 +++++++++++++++++
4 files changed, 1145 insertions(+), 16 deletions(-)
create mode 100755 tools/testing/selftests/net/srv6_encap_lookup_l3vpn_test.sh
--
2.20.1
^ permalink raw reply
* Re: [PATCH] net: hinic: validate firmware image section bounds
From: Simon Horman @ 2026-07-11 16:21 UTC (permalink / raw)
To: Pengpeng Hou
Cc: Cai Huoqing, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, netdev, linux-kernel
In-Reply-To: <20260706093455.81168-1-pengpeng@iscas.ac.cn>
On Mon, Jul 06, 2026 at 05:34:55PM +0800, Pengpeng Hou wrote:
> The firmware update path copies each section from the image with an offset
> and length supplied by the firmware header. The image validator only
> checked the sum of section lengths and used fw_len + header_size to check
> the file size, but it did not prove that each section offset and length
> fits in the firmware payload.
>
> Reject images with a truncated header, avoid the fw_len plus header-size
> addition, validate each section type before it is used as a bit index, and
> prove every section range with offset <= fw_len and len <= fw_len - offset.
>
> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
> ---
> .../net/ethernet/huawei/hinic/hinic_devlink.c | 57 ++++++++++++++++---
> 1 file changed, 49 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/ethernet/huawei/hinic/hinic_devlink.c b/drivers/net/ethernet/huawei/hinic/hinic_devlink.c
> index c977c43e5d5a..0c06fbdadad3 100644
> --- a/drivers/net/ethernet/huawei/hinic/hinic_devlink.c
> +++ b/drivers/net/ethernet/huawei/hinic/hinic_devlink.c
> @@ -20,14 +20,25 @@
> #include "hinic_devlink.h"
> #include "hinic_hw_dev.h"
>
> +static bool hinic_fw_section_valid(u32 fw_len, u32 offset, u32 len)
> +{
> + return offset <= fw_len && len <= fw_len - offset;
> +}
> +
> static bool check_image_valid(struct hinic_devlink_priv *priv, const u8 *buf,
> - u32 image_size, struct host_image_st *host_image)
> + size_t image_size, struct host_image_st *host_image)
> {
> - struct fw_image_st *fw_image = NULL;
> + const struct fw_image_st *fw_image;
I don't think the const update is strictly related to this patch,
and thus doesn't belong in this patch.
> u32 len = 0;
> u32 i;
>
> - fw_image = (struct fw_image_st *)buf;
> + if (image_size < UPDATEFW_IMAGE_HEAD_SIZE) {
> + dev_err(&priv->hwdev->hwif->pdev->dev,
> + "Wrong image size read from file\n");
> + return false;
> + }
> +
> + fw_image = (const struct fw_image_st *)buf;
>
> if (fw_image->fw_magic != HINIC_MAGIC_NUM) {
> dev_err(&priv->hwdev->hwif->pdev->dev, "Wrong fw_magic read from file, fw_magic: 0x%x\n",
> @@ -41,14 +52,44 @@ static bool check_image_valid(struct hinic_devlink_priv *priv, const u8 *buf,
> return false;
> }
>
> + if (fw_image->fw_len != image_size - UPDATEFW_IMAGE_HEAD_SIZE) {
> + dev_err(&priv->hwdev->hwif->pdev->dev,
> + "Wrong data size read from file\n");
> + return false;
> + }
> +
> for (i = 0; i < fw_image->fw_info.fw_section_cnt; i++) {
> - len += fw_image->fw_section_info[i].fw_section_len;
> - host_image->image_section_info[i] = fw_image->fw_section_info[i];
> + const struct fw_section_info_st *section =
> + &fw_image->fw_section_info[i];
> +
> + if (section->fw_section_type >= FILE_TYPE_TOTAL_NUM) {
> + dev_err(&priv->hwdev->hwif->pdev->dev,
> + "Wrong section type read from file: %u\n",
> + section->fw_section_type);
> + return false;
> + }
> +
> + if (!hinic_fw_section_valid(fw_image->fw_len,
> + section->fw_section_offset,
> + section->fw_section_len)) {
> + dev_err(&priv->hwdev->hwif->pdev->dev,
> + "Wrong section size read from file\n");
> + return false;
> + }
> +
> + if (section->fw_section_len > fw_image->fw_len - len) {
> + dev_err(&priv->hwdev->hwif->pdev->dev,
> + "Wrong data size read from file\n");
> + return false;
> + }
> +
> + len += section->fw_section_len;
> + host_image->image_section_info[i] = *section;
> }
>
> - if (len != fw_image->fw_len ||
> - (fw_image->fw_len + UPDATEFW_IMAGE_HEAD_SIZE) != image_size) {
> - dev_err(&priv->hwdev->hwif->pdev->dev, "Wrong data size read from file\n");
> + if (len != fw_image->fw_len) {
> + dev_err(&priv->hwdev->hwif->pdev->dev,
> + "Wrong data size read from file\n");
Likewise, the update to the dev_err() call, as distinct from the update
to the condition that precedes it, appears to be a whitespace cleanup.
If so, it doesn't belong in this patch.
> return false;
> }
>
--
pw-bot: changes-requested
^ permalink raw reply
* Re: [PATCH v9 11/14] net: ipa: Switch to generic PAS TZ APIs
From: Bjorn Andersson @ 2026-07-11 16:06 UTC (permalink / raw)
To: Sumit Garg
Cc: konradybcio, linux-arm-msm, devicetree, dri-devel, freedreno,
linux-media, netdev, linux-wireless, ath12k, linux-remoteproc,
robh, krzk+dt, conor+dt, robin.clark, sean, akhilpo, lumag,
abhinav.kumar, jesszhan0024, marijn.suijten, airlied, simona,
vikash.garodia, bod, mchehab, elder, andrew+netdev, davem,
edumazet, kuba, pabeni, jjohnson, mathieu.poirier,
trilokkumar.soni, mukesh.ojha, pavan.kondeti, jorge.ramirez,
tonyh, vignesh.viswanathan, srinivas.kandagatla, amirreza.zarrabi,
jenswi, op-tee, apurupa, skare, linux-kernel, Sumit Garg,
Alex Elder, Konrad Dybcio
In-Reply-To: <20260702115835.167602-12-sumit.garg@kernel.org>
On Thu, Jul 02, 2026 at 05:28:27PM +0530, Sumit Garg wrote:
> From: Sumit Garg <sumit.garg@oss.qualcomm.com>
>
> Switch ipa client driver over to generic PAS TZ APIs. Generic PAS TZ
> service allows to support multiple TZ implementation backends like QTEE
> based SCM PAS service, OP-TEE based PAS service and any further future TZ
> backend service.
>
Please find an immutable branch with the dependencies for this patch at:
https://git.kernel.org/pub/scm/linux/kernel/git/qcom/linux.git 20260702115835.167602-2-sumit.garg@kernel.org
Alternatively, if you think there will be no conflicting patches in the
time leading up to next merge window provide an Ack and I can pick this
through the qcom tree.
Thanks,
Bjorn
> Reviewed-by: Alex Elder <elder@riscstar.com>
> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> Signed-off-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
> ---
> drivers/net/ipa/Kconfig | 2 +-
> drivers/net/ipa/ipa_main.c | 13 ++++++++-----
> 2 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/ipa/Kconfig b/drivers/net/ipa/Kconfig
> index 01d219d3760c..a9aff1b7977d 100644
> --- a/drivers/net/ipa/Kconfig
> +++ b/drivers/net/ipa/Kconfig
> @@ -6,7 +6,7 @@ config QCOM_IPA
> depends on QCOM_RPROC_COMMON || (QCOM_RPROC_COMMON=n && COMPILE_TEST)
> depends on QCOM_AOSS_QMP || QCOM_AOSS_QMP=n
> select QCOM_MDT_LOADER
> - select QCOM_SCM
> + select QCOM_PAS
> select QCOM_QMI_HELPERS
> help
> Choose Y or M here to include support for the Qualcomm
> diff --git a/drivers/net/ipa/ipa_main.c b/drivers/net/ipa/ipa_main.c
> index 788dd99af2a4..3cd9e44680e9 100644
> --- a/drivers/net/ipa/ipa_main.c
> +++ b/drivers/net/ipa/ipa_main.c
> @@ -14,7 +14,7 @@
> #include <linux/pm_runtime.h>
> #include <linux/types.h>
>
> -#include <linux/firmware/qcom/qcom_scm.h>
> +#include <linux/firmware/qcom/qcom_pas.h>
> #include <linux/soc/qcom/mdt_loader.h>
>
> #include "ipa.h"
> @@ -624,10 +624,13 @@ static int ipa_firmware_load(struct device *dev)
> }
>
> ret = qcom_mdt_load(dev, fw, path, IPA_PAS_ID, virt, phys, size, NULL);
> - if (ret)
> + if (ret) {
> dev_err(dev, "error %d loading \"%s\"\n", ret, path);
> - else if ((ret = qcom_scm_pas_auth_and_reset(IPA_PAS_ID)))
> - dev_err(dev, "error %d authenticating \"%s\"\n", ret, path);
> + } else {
> + ret = qcom_pas_auth_and_reset(IPA_PAS_ID);
> + if (ret)
> + dev_err(dev, "error %d authenticating \"%s\"\n", ret, path);
> + }
>
> memunmap(virt);
> out_release_firmware:
> @@ -758,7 +761,7 @@ static enum ipa_firmware_loader ipa_firmware_loader(struct device *dev)
> return IPA_LOADER_INVALID;
> out_self:
> /* We need Trust Zone to load firmware; make sure it's available */
> - if (qcom_scm_is_available())
> + if (qcom_pas_is_available())
> return IPA_LOADER_SELF;
>
> return IPA_LOADER_DEFER;
> --
> 2.53.0
>
^ permalink raw reply
* Re: [PATCH v9 09/14] media: qcom: Switch to generic PAS TZ APIs
From: Bjorn Andersson @ 2026-07-11 16:05 UTC (permalink / raw)
To: Sumit Garg
Cc: konradybcio, linux-arm-msm, devicetree, dri-devel, freedreno,
linux-media, netdev, linux-wireless, ath12k, linux-remoteproc,
robh, krzk+dt, conor+dt, robin.clark, sean, akhilpo, lumag,
abhinav.kumar, jesszhan0024, marijn.suijten, airlied, simona,
vikash.garodia, bod, mchehab, elder, andrew+netdev, davem,
edumazet, kuba, pabeni, jjohnson, mathieu.poirier,
trilokkumar.soni, mukesh.ojha, pavan.kondeti, jorge.ramirez,
tonyh, vignesh.viswanathan, srinivas.kandagatla, amirreza.zarrabi,
jenswi, op-tee, apurupa, skare, linux-kernel, Sumit Garg,
Konrad Dybcio
In-Reply-To: <20260702115835.167602-10-sumit.garg@kernel.org>
On Thu, Jul 02, 2026 at 05:28:25PM +0530, Sumit Garg wrote:
> From: Sumit Garg <sumit.garg@oss.qualcomm.com>
>
> Switch qcom media client drivers over to generic PAS TZ APIs. Generic PAS
> TZ service allows to support multiple TZ implementation backends like QTEE
> based SCM PAS service, OP-TEE based PAS service and any further future TZ
> backend service.
>
Please find an immutable branch with the dependencies for this patch at:
https://git.kernel.org/pub/scm/linux/kernel/git/qcom/linux.git 20260702115835.167602-2-sumit.garg@kernel.org
Alternatively, if you think there will be no conflicting patches in the
time leading up to next merge window provide an Ack and I can pick this
through the qcom tree.
Thanks,
Bjorn
> Reviewed-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> Tested-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com> # Lemans
> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> Signed-off-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
> ---
> drivers/media/platform/qcom/iris/Kconfig | 27 ++++++++++---------
> .../media/platform/qcom/iris/iris_firmware.c | 9 ++++---
> drivers/media/platform/qcom/venus/Kconfig | 1 +
> drivers/media/platform/qcom/venus/firmware.c | 11 ++++----
> 4 files changed, 26 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/iris/Kconfig b/drivers/media/platform/qcom/iris/Kconfig
> index af78a1775937..388c9bbc8136 100644
> --- a/drivers/media/platform/qcom/iris/Kconfig
> +++ b/drivers/media/platform/qcom/iris/Kconfig
> @@ -1,14 +1,15 @@
> config VIDEO_QCOM_IRIS
> - tristate "Qualcomm iris V4L2 decoder driver"
> - depends on VIDEO_DEV
> - depends on ARCH_QCOM || COMPILE_TEST
> - select V4L2_MEM2MEM_DEV
> - select QCOM_MDT_LOADER
> - select QCOM_SCM
> - select QCOM_UBWC_CONFIG
> - select VIDEOBUF2_DMA_CONTIG
> - help
> - This is a V4L2 driver for Qualcomm iris video accelerator
> - hardware. It accelerates decoding operations on various
> - Qualcomm SoCs.
> - To compile this driver as a module choose m here.
> + tristate "Qualcomm iris V4L2 decoder driver"
> + depends on VIDEO_DEV
> + depends on ARCH_QCOM || COMPILE_TEST
> + select V4L2_MEM2MEM_DEV
> + select QCOM_MDT_LOADER
> + select QCOM_SCM
> + select QCOM_PAS
> + select QCOM_UBWC_CONFIG
> + select VIDEOBUF2_DMA_CONTIG
> + help
> + This is a V4L2 driver for Qualcomm iris video accelerator
> + hardware. It accelerates decoding operations on various
> + Qualcomm SoCs.
> + To compile this driver as a module choose m here.
> diff --git a/drivers/media/platform/qcom/iris/iris_firmware.c b/drivers/media/platform/qcom/iris/iris_firmware.c
> index 1a476146d758..ea9654dd679e 100644
> --- a/drivers/media/platform/qcom/iris/iris_firmware.c
> +++ b/drivers/media/platform/qcom/iris/iris_firmware.c
> @@ -4,6 +4,7 @@
> */
>
> #include <linux/firmware.h>
> +#include <linux/firmware/qcom/qcom_pas.h>
> #include <linux/firmware/qcom/qcom_scm.h>
> #include <linux/of_address.h>
> #include <linux/of_reserved_mem.h>
> @@ -80,7 +81,7 @@ int iris_fw_load(struct iris_core *core)
> return -ENOMEM;
> }
>
> - ret = qcom_scm_pas_auth_and_reset(IRIS_PAS_ID);
> + ret = qcom_pas_auth_and_reset(IRIS_PAS_ID);
> if (ret) {
> dev_err(core->dev, "auth and reset failed: %d\n", ret);
> return ret;
> @@ -94,7 +95,7 @@ int iris_fw_load(struct iris_core *core)
> cp_config->cp_nonpixel_size);
> if (ret) {
> dev_err(core->dev, "qcom_scm_mem_protect_video_var failed: %d\n", ret);
> - qcom_scm_pas_shutdown(IRIS_PAS_ID);
> + qcom_pas_shutdown(IRIS_PAS_ID);
> return ret;
> }
> }
> @@ -104,10 +105,10 @@ int iris_fw_load(struct iris_core *core)
>
> int iris_fw_unload(struct iris_core *core)
> {
> - return qcom_scm_pas_shutdown(IRIS_PAS_ID);
> + return qcom_pas_shutdown(IRIS_PAS_ID);
> }
>
> int iris_set_hw_state(struct iris_core *core, bool resume)
> {
> - return qcom_scm_set_remote_state(resume, 0);
> + return qcom_pas_set_remote_state(resume, 0);
> }
> diff --git a/drivers/media/platform/qcom/venus/Kconfig b/drivers/media/platform/qcom/venus/Kconfig
> index 63ee8c78dc6d..7997b8aa427a 100644
> --- a/drivers/media/platform/qcom/venus/Kconfig
> +++ b/drivers/media/platform/qcom/venus/Kconfig
> @@ -6,6 +6,7 @@ config VIDEO_QCOM_VENUS
> select OF_DYNAMIC if ARCH_QCOM
> select QCOM_MDT_LOADER
> select QCOM_SCM
> + select QCOM_PAS
> select VIDEOBUF2_DMA_CONTIG
> select V4L2_MEM2MEM_DEV
> help
> diff --git a/drivers/media/platform/qcom/venus/firmware.c b/drivers/media/platform/qcom/venus/firmware.c
> index 1de7436713ed..3a38ff985822 100644
> --- a/drivers/media/platform/qcom/venus/firmware.c
> +++ b/drivers/media/platform/qcom/venus/firmware.c
> @@ -12,6 +12,7 @@
> #include <linux/of_reserved_mem.h>
> #include <linux/platform_device.h>
> #include <linux/of_device.h>
> +#include <linux/firmware/qcom/qcom_pas.h>
> #include <linux/firmware/qcom/qcom_scm.h>
> #include <linux/sizes.h>
> #include <linux/soc/qcom/mdt_loader.h>
> @@ -58,7 +59,7 @@ int venus_set_hw_state(struct venus_core *core, bool resume)
> int ret;
>
> if (core->use_tz) {
> - ret = qcom_scm_set_remote_state(resume, 0);
> + ret = qcom_pas_set_remote_state(resume, 0);
> if (resume && ret == -EINVAL)
> ret = 0;
> return ret;
> @@ -218,7 +219,7 @@ int venus_boot(struct venus_core *core)
> int ret;
>
> if (!IS_ENABLED(CONFIG_QCOM_MDT_LOADER) ||
> - (core->use_tz && !qcom_scm_is_available()))
> + (core->use_tz && !qcom_pas_is_available()))
> return -EPROBE_DEFER;
>
> ret = of_property_read_string_index(dev->of_node, "firmware-name", 0,
> @@ -236,7 +237,7 @@ int venus_boot(struct venus_core *core)
> core->fw.mem_phys = mem_phys;
>
> if (core->use_tz)
> - ret = qcom_scm_pas_auth_and_reset(VENUS_PAS_ID);
> + ret = qcom_pas_auth_and_reset(VENUS_PAS_ID);
> else
> ret = venus_boot_no_tz(core, mem_phys, mem_size);
>
> @@ -259,7 +260,7 @@ int venus_boot(struct venus_core *core)
> res->cp_nonpixel_start,
> res->cp_nonpixel_size);
> if (ret) {
> - qcom_scm_pas_shutdown(VENUS_PAS_ID);
> + qcom_pas_shutdown(VENUS_PAS_ID);
> dev_err(dev, "set virtual address ranges fail (%d)\n",
> ret);
> return ret;
> @@ -274,7 +275,7 @@ int venus_shutdown(struct venus_core *core)
> int ret;
>
> if (core->use_tz)
> - ret = qcom_scm_pas_shutdown(VENUS_PAS_ID);
> + ret = qcom_pas_shutdown(VENUS_PAS_ID);
> else
> ret = venus_shutdown_no_tz(core);
>
> --
> 2.53.0
>
^ permalink raw reply
* Re: (subset) [PATCH v9 00/14] firmware: qcom: Add OP-TEE PAS service support
From: Bjorn Andersson @ 2026-07-11 16:03 UTC (permalink / raw)
To: Sumit Garg
Cc: konradybcio, linux-arm-msm, devicetree, dri-devel, freedreno,
linux-media, netdev, linux-wireless, ath12k, linux-remoteproc,
robh, krzk+dt, conor+dt, robin.clark, sean, akhilpo, lumag,
abhinav.kumar, jesszhan0024, marijn.suijten, airlied, simona,
vikash.garodia, bod, mchehab, elder, andrew+netdev, davem,
edumazet, kuba, pabeni, jjohnson, mathieu.poirier,
trilokkumar.soni, mukesh.ojha, pavan.kondeti, jorge.ramirez,
tonyh, vignesh.viswanathan, srinivas.kandagatla, amirreza.zarrabi,
jenswi, op-tee, apurupa, skare, linux-kernel, Sumit Garg
In-Reply-To: <alDhkHVnzReCgU6H@sumit-xelite>
On Fri, Jul 10, 2026 at 05:42:00PM +0530, Sumit Garg wrote:
> Hi Bjorn,
>
> On Thu, Jul 09, 2026 at 02:32:39PM -0500, Bjorn Andersson wrote:
> >
> > On Thu, 02 Jul 2026 17:28:16 +0530, Sumit Garg wrote:
> > > From: Sumit Garg <sumit.garg@oss.qualcomm.com>
> > >
> > > Qcom platforms has the legacy of using non-standard SCM calls
> > > splintered over the various kernel drivers. These SCM calls aren't
> > > compliant with the standard SMC calling conventions which is a
> > > prerequisite to enable migration to the FF-A specifications from Arm.
> > >
> > > [...]
> >
> > Applied, thanks!
> >
> > [01/14] firmware: qcom: Add a generic PAS service
> > commit: 08314e7c2c38b9ae6a5e01c58ed10a950859404d
> > [02/14] firmware: qcom_scm: Migrate to generic PAS service
> > commit: 5c1a2975d23c51c01aca51945d0f10a4ee4c9020
> > [03/14] firmware: qcom: Add a PAS TEE service
> > commit: b6f7978da0c4d26fe465aa6634f5a0b48f900de0
> > [14/14] MAINTAINERS: Add maintainer entry for Qualcomm PAS TZ service
> > commit: 6701259025d49139131a0eb2257659a066dcca22
> >
> > This is available as an immutable branch, for other subsystems to pull at:
> > https://git.kernel.org/pub/scm/linux/kernel/git/qcom/linux.git 20260702115835.167602-2-sumit.garg@kernel.org
> >
> >
> > [04/14] remoteproc: qcom_q6v5_pas: Switch over to generic PAS TZ APIs
> > commit: 254030af0d81b12b7624d9ce85c6bdd3171629c6
> > [05/14] remoteproc: qcom_q6v5_mss: Switch to generic PAS TZ APIs
> > commit: f3b1357673ddb37ae8b9a8fe44df73cbd2a519c5
> > [06/14] remoteproc: qcom_wcnss: Switch to generic PAS TZ APIs
> > commit: ea3b5245f5deba916320b32a8e6510a74c034c17
> > [07/14] remoteproc: qcom: Select QCOM_PAS generic service
> > commit: c4383254ac7a529736577e304176a10371c2ee0b
> >
>
> Thanks for picking the partial set although I expected for you to pick
> the entire set given acks from all the other subsystem maintainers. Let
> me know how we should proceed further.
>
That would be desirable, so that I can also merge the cleanup patch at
the tail end there.
I did miss Dmitry's ack on the drm patch - that one I could have merged,
but I don't see acks from media and net maintainers. I'm fine either way
(them providing acks, or merging the immutable branch and respective
change).
As you can see Jeff did the latter already. Thanks Jeff!
This leaves us with the cleanup patch (13), which I think we can pick
for v7.3-rc2.
Regards,
Bjorn
^ permalink raw reply
* Re: [PATCH] virtio_net: validate device stats reply records before use
From: Michael S. Tsirkin @ 2026-07-11 15:52 UTC (permalink / raw)
To: Michael Bommarito
Cc: Jason Wang, Xuan Zhuo, Eugenio Pérez, Andrew Lunn,
Jakub Kicinski, Paolo Abeni, virtualization, netdev, linux-kernel,
stable
In-Reply-To: <CAJJ9bXwUtQ3pHqZ=AMuwaNLs16pmujiMdeBQtB5kFc6JjM-Pug@mail.gmail.com>
On Sat, Jul 11, 2026 at 11:29:56AM -0400, Michael Bommarito wrote:
> On Sat, Jul 11, 2026 at 11:20 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> > Why does it "matter most", or at all, there?
> > Host can always deny guest service. In fact, this is how cloud vendors
> > charge their clients, by denying service to whoever did not pay them.
> ...
> > I'm all for making things easier to debug even when the device is buggy.
> > But I'm not inclined to add tons of hard to maintain code to
> > that end, and I would be worried broken hosts will come to
> > rely on drivers working around them.
>
> I am always confused by the CoCo threat model to be honest,
Confidential computing? It's vague at points, given the term covers a
lot of different hardware. But one thing is clear - it's about
confidentiality. DoS by host is empathically outside the threat model.
On any virtualization platform I know without exception,
host can just exit the VM, done, service denied.
> since it
> seems like some people care a lot about maximalist reliance on the
> contract and other people are more practical about how many other
> vectors exist anyway.
I don't really know what "vectors" or "the contract" are here.
Making a guest recover from a misbehaving device has as much a chance
to reduce security as increase it. So the only benefit is
robustness for users/developers, not security. And that
has to be weighted against the maintainance cost of the change.
This one is too costly, I judge.
> No hard feelings if you want to NACK, but at
> least it's documented publicly now for people to consider.
>
> Thanks,
> Mike
^ permalink raw reply
* Re: [PATCH net] selftests: netconsole: only restore MAC when it changed on resume
From: Matthieu Baerts @ 2026-07-11 15:34 UTC (permalink / raw)
To: Andre Carvalho
Cc: netdev, linux-kselftest, linux-kernel, Breno Leitao, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Shuah Khan
In-Reply-To: <20260710-netcons-mac-reload-v1-1-3fb1bcc70b4a@gmail.com>
Hi Andre,
On 11/07/2026 00:19, Andre Carvalho wrote:
> The "mac" bind mode reactivation downs the interface, restores the saved
> MAC and renames it to trigger a target resume. This assumes the recreated
> interface comes back with a different MAC, which is true under
> MACAddressPolicy=none (as on the Netdev CI) but not when MACs are
> persistent. In the persistent case netconsole resumes the target on its
> own, and the down/restore/rename flow instead drops it and fails the test.
>
> Guard the block on the MAC having actually changed so the test passes
> under both policies.
Thank you for the patch, it fixed the issue on my side when not forcing
MACAddressPolicy=none.
Tested-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
Cheers,
Matt
^ permalink raw reply
* Re: [PATCH] netxen: validate unified ROM directory bounds
From: Simon Horman @ 2026-07-11 15:30 UTC (permalink / raw)
To: pengpeng
Cc: Simon Horman, manishc, rahulv, GR-Linux-NIC-Dev, andrew+netdev,
davem, edumazet, kuba, pabeni, netdev, linux-kernel
In-Reply-To: <20260706093702.81687-1-pengpeng@iscas.ac.cn>
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
---
Subject: netxen: validate unified ROM directory bounds
This patch updates the unified ROM parser for the netxen driver to use safer
range validation helpers. It calculates bounds using division and subtraction
to avoid integer overflow vulnerabilities when validating directory entries,
bootloader, firmware, and product tables.
> diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c b/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
> index 8bc4e2b69569e..5722c55fa0cce 100644
> --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
> +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
[ ... ]
> @@ -561,20 +561,55 @@ int netxen_pinit_from_rom(struct netxen_adapter *adapter)
[ ... ]
> +static struct uni_table_desc *
> +nx_get_table_desc(const u8 *unirom, size_t fw_size, int section)
> {
> struct uni_table_desc *directory = (struct uni_table_desc *) &unirom[0];
> - __le32 entries = cpu_to_le32(directory->num_entries);
> + u32 entries = cpu_to_le32(directory->num_entries);
> + u32 entry_size = cpu_to_le32(directory->entry_size);
> + u32 findex = cpu_to_le32(directory->findex);
> + u32 i;
>
> - for (i = 0; i < entries; i++) {
> + if (entry_size < NX_UNI_DIR_ENTRY_MIN_SIZE ||
> + !netxen_rom_table_valid(fw_size, findex, entries, entry_size))
> + return NULL;
>
> - __le32 offs = cpu_to_le32(directory->findex) +
> - (i * cpu_to_le32(directory->entry_size));
> - __le32 tab_type = cpu_to_le32(*((u32 *)&unirom[offs] + 8));
> + for (i = 0; i < entries; i++) {
> + size_t offs = findex + (size_t)i * entry_size;
> + u32 tab_type = cpu_to_le32(*((u32 *)&unirom[offs] +
> + NX_UNI_DIR_TYPE_OFF));
[Severity: High]
This isn't a bug introduced by this patch, but can this cast trigger an
unaligned access panic?
If findex or entry_size provided by the firmware header are unaligned,
offs will not be a multiple of 4. Casting &unirom[offs] directly to a
32-bit integer pointer and dereferencing it could fault on architectures that
do not support unaligned memory accesses natively.
>
> if (tab_type == section)
> - return (struct uni_table_desc *) &unirom[offs];
> + return (struct uni_table_desc *)&unirom[offs];
> }
>
> return NULL;
[ ... ]
> @@ -611,30 +644,30 @@ netxen_nic_validate_bootld(struct netxen_adapter *adapter)
[ ... ]
> - offs = cpu_to_le32(tab_desc->findex) +
> - (cpu_to_le32(tab_desc->entry_size) * (idx));
> + offs = findex + (size_t)entry_size * idx;
> descr = (struct uni_data_desc *)&unirom[offs];
> + data_off = cpu_to_le32(descr->findex);
> + data_len = cpu_to_le32(descr->size);
>
> - data_size = cpu_to_le32(descr->findex) + cpu_to_le32(descr->size);
> -
> - if (adapter->fw->size < data_size)
> + if (!netxen_rom_range_valid(adapter->fw->size, data_off, data_len))
> return -EINVAL;
[Severity: High]
This is a pre-existing issue, but does netxen_load_firmware() ignore this
validated data_len?
It appears netxen_load_firmware() uses a hardcoded copy size:
netxen_load_firmware() {
...
size = (NETXEN_IMAGE_START - NETXEN_BOOTLD_START) / 8;
ptr64 = (u64 *)nx_get_bootld_offs(adapter);
for (i = 0; i < size; i++) {
data = cpu_to_le64(ptr64[i]);
...
}
If data_len is small, couldn't this hardcoded size cause an out-of-bounds slab
memory read past the end of the firmware file data?
>
> return 0;
> @@ -646,29 +679,30 @@ netxen_nic_validate_fw(struct netxen_adapter *adapter)
[ ... ]
> - offs = cpu_to_le32(tab_desc->findex) +
> - (cpu_to_le32(tab_desc->entry_size) * (idx));
> + offs = findex + (size_t)entry_size * idx;
> descr = (struct uni_data_desc *)&unirom[offs];
> - data_size = cpu_to_le32(descr->findex) + cpu_to_le32(descr->size);
> + data_off = cpu_to_le32(descr->findex);
> + data_len = cpu_to_le32(descr->size);
>
> - if (adapter->fw->size < data_size)
> + if (!netxen_rom_range_valid(adapter->fw->size, data_off, data_len))
> return -EINVAL;
[Severity: High]
This is a pre-existing issue, but does this validation need to enforce a
minimum length for data_len (size)?
In nx_get_fw_version(), the version string pointer calculation relies on size:
nx_get_fw_version() {
...
ver_str = fw->data + cpu_to_le32(fw_data_desc->findex) +
cpu_to_le32(fw_data_desc->size) - 17;
for (i = 0; i < 12; i++) {
if (!strncmp(&ver_str[i], "REV=", 4)) {
...
}
If a malformed firmware image provides an entry where size is less than 17,
wouldn't the subtraction wrap around and result in a massive out-of-bounds
memory read?
Additionally, in netxen_load_firmware(), if the firmware data_len is not a
multiple of 8, it seems the remainder is handled unconditionally by reading a
full 8-byte word:
netxen_load_firmware() {
...
size = (__force u32)nx_get_fw_size(adapter) % 8;
if (size) {
data = cpu_to_le64(ptr64[i]);
...
}
If fw->size is exactly data_off + data_len, wouldn't reading an 8-byte chunk
at an unaligned remainder boundary access up to 7 bytes past the end of the
fw->data buffer?
>
> return 0;
> @@ -682,39 +716,37 @@ netxen_nic_validate_product_offs(struct netxen_adapter *adapter)
[ ... ]
> nomn:
> for (i = 0; i < entries; i++) {
> -
> - __le32 flags, file_chiprev, offs;
> + size_t offs;
> + __le32 flags, file_chiprev;
> u8 chiprev = adapter->ahw.revision_id;
> uint32_t flagbit;
>
> - offs = cpu_to_le32(ptab_descr->findex) +
> - (i * cpu_to_le32(ptab_descr->entry_size));
> + offs = findex + (size_t)i * entry_size;
> flags = cpu_to_le32(*((int *)&unirom[offs] + NX_UNI_FLAGS_OFF));
[Severity: High]
This isn't a bug introduced by this patch, but can this cast to (int *) also
trigger an unaligned access panic if the firmware supplies an unaligned
findex or entry_size?
> file_chiprev = cpu_to_le32(*((int *)&unirom[offs] +
> - NX_UNI_CHIP_REV_OFF));
> + NX_UNI_CHIP_REV_OFF));
>
> flagbit = mn_present ? 1 : 2;
>
--
pw-bot: changes-requested
^ permalink raw reply
* Re: [PATCH] virtio_net: validate device stats reply records before use
From: Michael Bommarito @ 2026-07-11 15:29 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Jason Wang, Xuan Zhuo, Eugenio Pérez, Andrew Lunn,
Jakub Kicinski, Paolo Abeni, virtualization, netdev, linux-kernel,
stable
In-Reply-To: <20260711111503-mutt-send-email-mst@kernel.org>
On Sat, Jul 11, 2026 at 11:20 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> Why does it "matter most", or at all, there?
> Host can always deny guest service. In fact, this is how cloud vendors
> charge their clients, by denying service to whoever did not pay them.
...
> I'm all for making things easier to debug even when the device is buggy.
> But I'm not inclined to add tons of hard to maintain code to
> that end, and I would be worried broken hosts will come to
> rely on drivers working around them.
I am always confused by the CoCo threat model to be honest, since it
seems like some people care a lot about maximalist reliance on the
contract and other people are more practical about how many other
vectors exist anyway. No hard feelings if you want to NACK, but at
least it's documented publicly now for people to consider.
Thanks,
Mike
^ permalink raw reply
* [PATCH 2/2] PCI: Replace pci_dev->broken_parity_status with accessors
From: Maurice Hieronymus @ 2026-07-11 15:21 UTC (permalink / raw)
To: Edward Cree, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Bjorn Helgaas, Justin Tee, Paul Ely,
James E.J. Bottomley, Martin K. Petersen, Juergen Gross,
Stefano Stabellini, Oleksandr Tyshchenko, Miguel Ojeda,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Daniel Almeida,
Tamir Duberstein, Alexandre Courbot, Onur Özkan,
Borislav Petkov, Tony Luck
Cc: Danilo Krummrich, rust-for-linux, netdev, linux-net-drivers,
linux-kernel, linux-pci, linux-scsi, xen-devel, linux-edac,
Maurice Hieronymus
In-Reply-To: <20260711-pci-dev-flags-v1-0-2fcf2811138c@mailbox.org>
`broken_parity_status` shares a C bitfield word in `struct pci_dev`
with many other bits. `broken_parity_status_store()` writes it from
sysfs at any time without taking any lock, so userspace can make it
race with every other writer of the same word, e.g. `pci_set_master()`
from a runtime PM resume path, and updates of neighboring bits can be
lost.
Move the bit into the `flags` bitmap modified with atomic bitops,
using the accessor pattern introduced by the previous commit.
Signed-off-by: Maurice Hieronymus <mhi@mailbox.org>
---
drivers/edac/edac_pci_sysfs.c | 4 ++--
drivers/pci/pci-sysfs.c | 4 ++--
include/linux/pci.h | 5 ++++-
3 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/edac/edac_pci_sysfs.c b/drivers/edac/edac_pci_sysfs.c
index 9f437f648e4e..fadc61235f1f 100644
--- a/drivers/edac/edac_pci_sysfs.c
+++ b/drivers/edac/edac_pci_sysfs.c
@@ -554,7 +554,7 @@ static void edac_pci_dev_parity_test(struct pci_dev *dev)
/* check the status reg for errors on boards NOT marked as broken
* if broken, we cannot trust any of the status bits
*/
- if (status && !dev->broken_parity_status) {
+ if (status && !pci_dev_broken_parity_status(dev)) {
if (status & (PCI_STATUS_SIG_SYSTEM_ERROR)) {
edac_printk(KERN_CRIT, EDAC_PCI,
"Signaled System Error on %s\n",
@@ -593,7 +593,7 @@ static void edac_pci_dev_parity_test(struct pci_dev *dev)
/* check the secondary status reg for errors,
* on NOT broken boards
*/
- if (status && !dev->broken_parity_status) {
+ if (status && !pci_dev_broken_parity_status(dev)) {
if (status & (PCI_STATUS_SIG_SYSTEM_ERROR)) {
edac_printk(KERN_CRIT, EDAC_PCI, "Bridge "
"Signaled System Error on %s\n",
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 5ec0b245a69b..5e094d1e23e3 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -80,7 +80,7 @@ static ssize_t broken_parity_status_show(struct device *dev,
char *buf)
{
struct pci_dev *pdev = to_pci_dev(dev);
- return sysfs_emit(buf, "%u\n", pdev->broken_parity_status);
+ return sysfs_emit(buf, "%u\n", pci_dev_broken_parity_status(pdev));
}
static ssize_t broken_parity_status_store(struct device *dev,
@@ -93,7 +93,7 @@ static ssize_t broken_parity_status_store(struct device *dev,
if (kstrtoul(buf, 0, &val) < 0)
return -EINVAL;
- pdev->broken_parity_status = !!val;
+ pci_dev_assign_broken_parity_status(pdev, val);
return count;
}
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 9964646bdd46..fdcd9b1b7371 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -347,10 +347,13 @@ struct rcec_ea;
* bookkeeping state, maintained by pci_set_master(),
* pci_clear_master() and pci_disable_device(); modifying it
* does not itself change the hardware state.
+ * @PCI_DEV_FLAG_BROKEN_PARITY_STATUS: Device generates false positive
+ * parity errors; set via sysfs.
* @PCI_DEV_FLAG_COUNT: Number of defined struct_pci_dev_flags.
*/
enum struct_pci_dev_flags {
PCI_DEV_FLAG_BUSMASTER = 0,
+ PCI_DEV_FLAG_BROKEN_PARITY_STATUS = 1,
PCI_DEV_FLAG_COUNT
};
@@ -482,7 +485,6 @@ struct pci_dev {
unsigned int no_msi:1; /* May not use MSI */
unsigned int block_cfg_access:1; /* Config space access blocked */
- unsigned int broken_parity_status:1; /* Generates false positive parity */
unsigned int irq_reroute_variant:2; /* Needs IRQ rerouting variant */
unsigned int msi_enabled:1;
unsigned int msix_enabled:1;
@@ -626,6 +628,7 @@ static inline void pci_dev_assign_##accessor_name(struct pci_dev *pdev, bool val
}
__create_pci_dev_flag_accessors(busmaster, PCI_DEV_FLAG_BUSMASTER);
+__create_pci_dev_flag_accessors(broken_parity_status, PCI_DEV_FLAG_BROKEN_PARITY_STATUS);
#undef __create_pci_dev_flag_accessors
--
2.51.2
^ permalink raw reply related
* [PATCH 1/2] PCI: Replace pci_dev->is_busmaster with accessors
From: Maurice Hieronymus @ 2026-07-11 15:21 UTC (permalink / raw)
To: Edward Cree, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Bjorn Helgaas, Justin Tee, Paul Ely,
James E.J. Bottomley, Martin K. Petersen, Juergen Gross,
Stefano Stabellini, Oleksandr Tyshchenko, Miguel Ojeda,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Daniel Almeida,
Tamir Duberstein, Alexandre Courbot, Onur Özkan,
Borislav Petkov, Tony Luck
Cc: Danilo Krummrich, rust-for-linux, netdev, linux-net-drivers,
linux-kernel, linux-pci, linux-scsi, xen-devel, linux-edac,
Maurice Hieronymus
In-Reply-To: <20260711-pci-dev-flags-v1-0-2fcf2811138c@mailbox.org>
`is_busmaster` is one bit of a ~60-bit C bitfield in `struct pci_dev`.
Bits sharing a bitfield word must not be modified concurrently, but its
writers take no common lock: `pci_set_master()` can run without the
device lock (e.g. from runtime PM resume paths), `pci_disable_device()`
clears the bit, and other bits in the same word are written from
entirely different contexts, e.g. `broken_parity_status` from sysfs.
Concurrent read-modify-write cycles of the shared word can then lose
updates.
Move `is_busmaster` into a new `flags` bitmap modified with atomic
bitops and accessed through generated accessor functions, following the
example of commit a7cc262a1135 ("driver core: Replace dev->offline +
->offline_disabled with accessors"). More bitfield flags can follow the
same pattern later.
This also unblocks the Rust device enabling API rework [1], where a
guard object calls `pci_disable_device()` from contexts that may run
concurrently with `pci_set_master()`.
Link: https://lore.kernel.org/rust-for-linux/DJOEYVBS17MJ.1YD3TNGQBWHNK@kernel.org/ [1]
Suggested-by: Danilo Krummrich <dakr@kernel.org>
Cc: rust-for-linux@vger.kernel.org
Signed-off-by: Maurice Hieronymus <mhi@mailbox.org>
---
drivers/net/ethernet/sfc/falcon/farch.c | 2 +-
drivers/net/ethernet/sfc/siena/farch.c | 2 +-
drivers/pci/pci-driver.c | 2 +-
drivers/pci/pci.c | 6 ++---
drivers/scsi/lpfc/lpfc_init.c | 4 ++--
| 4 ++--
drivers/xen/xen-pciback/pciback_ops.c | 4 ++--
include/linux/pci.h | 37 ++++++++++++++++++++++++++++-
8 files changed, 48 insertions(+), 13 deletions(-)
diff --git a/drivers/net/ethernet/sfc/falcon/farch.c b/drivers/net/ethernet/sfc/falcon/farch.c
index 23d507a3820d..42594bd7e818 100644
--- a/drivers/net/ethernet/sfc/falcon/farch.c
+++ b/drivers/net/ethernet/sfc/falcon/farch.c
@@ -724,7 +724,7 @@ int ef4_farch_fini_dmaq(struct ef4_nic *efx)
/* Do not attempt to write to the NIC during EEH recovery */
if (efx->state != STATE_RECOVERY) {
/* Only perform flush if DMA is enabled */
- if (efx->pci_dev->is_busmaster) {
+ if (pci_dev_busmaster(efx->pci_dev)) {
efx->type->prepare_flush(efx);
rc = ef4_farch_do_flush(efx);
efx->type->finish_flush(efx);
diff --git a/drivers/net/ethernet/sfc/siena/farch.c b/drivers/net/ethernet/sfc/siena/farch.c
index 7613d7988894..f673af4c77b6 100644
--- a/drivers/net/ethernet/sfc/siena/farch.c
+++ b/drivers/net/ethernet/sfc/siena/farch.c
@@ -723,7 +723,7 @@ int efx_farch_fini_dmaq(struct efx_nic *efx)
/* Do not attempt to write to the NIC during EEH recovery */
if (efx->state != STATE_RECOVERY) {
/* Only perform flush if DMA is enabled */
- if (efx->pci_dev->is_busmaster) {
+ if (pci_dev_busmaster(efx->pci_dev)) {
efx->type->prepare_flush(efx);
rc = efx_farch_do_flush(efx);
efx->type->finish_flush(efx);
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index f36778e62ac1..412afa12a285 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -649,7 +649,7 @@ static int pci_pm_reenable_device(struct pci_dev *pci_dev)
* if the device was busmaster before the suspend, make it busmaster
* again
*/
- if (pci_dev->is_busmaster)
+ if (pci_dev_busmaster(pci_dev))
pci_set_master(pci_dev);
return retval;
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 77b17b13ee61..c4fd6fe6098d 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -2045,7 +2045,7 @@ static void pci_enable_bridge(struct pci_dev *dev)
pci_enable_bridge(bridge);
if (pci_is_enabled(dev)) {
- if (!dev->is_busmaster)
+ if (!pci_dev_busmaster(dev))
pci_set_master(dev);
return;
}
@@ -2205,7 +2205,7 @@ void pci_disable_device(struct pci_dev *dev)
do_pci_disable_device(dev);
- dev->is_busmaster = 0;
+ pci_dev_assign_busmaster(dev, false);
}
EXPORT_SYMBOL(pci_disable_device);
@@ -4120,7 +4120,7 @@ static void __pci_set_master(struct pci_dev *dev, bool enable)
enable ? "enabling" : "disabling");
pci_write_config_word(dev, PCI_COMMAND, cmd);
}
- dev->is_busmaster = enable;
+ pci_dev_assign_busmaster(dev, enable);
}
/**
diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c
index 82af59c913e9..08dc06e7dfc2 100644
--- a/drivers/scsi/lpfc/lpfc_init.c
+++ b/drivers/scsi/lpfc/lpfc_init.c
@@ -14398,7 +14398,7 @@ lpfc_io_slot_reset_s3(struct pci_dev *pdev)
pci_restore_state(pdev);
- if (pdev->is_busmaster)
+ if (pci_dev_busmaster(pdev))
pci_set_master(pdev);
spin_lock_irq(&phba->hbalock);
@@ -15251,7 +15251,7 @@ lpfc_io_slot_reset_s4(struct pci_dev *pdev)
*/
pci_save_state(pdev);
- if (pdev->is_busmaster)
+ if (pci_dev_busmaster(pdev))
pci_set_master(pdev);
spin_lock_irq(&phba->hbalock);
--git a/drivers/xen/xen-pciback/conf_space_header.c b/drivers/xen/xen-pciback/conf_space_header.c
index 8b50cbcbdfe1..59a89f915916 100644
--- a/drivers/xen/xen-pciback/conf_space_header.c
+++ b/drivers/xen/xen-pciback/conf_space_header.c
@@ -81,10 +81,10 @@ static int command_write(struct pci_dev *dev, int offset, u16 value, void *data)
dev_data->enable_intx = 0;
}
- if (!dev->is_busmaster && is_master_cmd(value)) {
+ if (!pci_dev_busmaster(dev) && is_master_cmd(value)) {
dev_dbg(&dev->dev, "set bus master\n");
pci_set_master(dev);
- } else if (dev->is_busmaster && !is_master_cmd(value)) {
+ } else if (pci_dev_busmaster(dev) && !is_master_cmd(value)) {
dev_dbg(&dev->dev, "clear bus master\n");
pci_clear_master(dev);
}
diff --git a/drivers/xen/xen-pciback/pciback_ops.c b/drivers/xen/xen-pciback/pciback_ops.c
index bfc186bf05bc..01f4705421c9 100644
--- a/drivers/xen/xen-pciback/pciback_ops.c
+++ b/drivers/xen/xen-pciback/pciback_ops.c
@@ -125,14 +125,14 @@ void xen_pcibk_reset_device(struct pci_dev *dev)
if (pci_is_enabled(dev))
pci_disable_device(dev);
- dev->is_busmaster = 0;
+ pci_dev_assign_busmaster(dev, false);
} else {
pci_read_config_word(dev, PCI_COMMAND, &cmd);
if (cmd & (PCI_COMMAND_INVALIDATE)) {
cmd &= ~(PCI_COMMAND_INVALIDATE);
pci_write_config_word(dev, PCI_COMMAND, cmd);
- dev->is_busmaster = 0;
+ pci_dev_assign_busmaster(dev, false);
}
}
}
diff --git a/include/linux/pci.h b/include/linux/pci.h
index ebb5b9d76360..9964646bdd46 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -336,6 +336,25 @@ struct pci_sriov;
struct pci_p2pdma;
struct rcec_ea;
+/**
+ * enum struct_pci_dev_flags - Flags in struct pci_dev
+ *
+ * Each flag has a set of accessor functions created via
+ * __create_pci_dev_flag_accessors() and must only be accessed through
+ * them.
+ *
+ * @PCI_DEV_FLAG_BUSMASTER: Bus mastering is enabled on the device. Pure
+ * bookkeeping state, maintained by pci_set_master(),
+ * pci_clear_master() and pci_disable_device(); modifying it
+ * does not itself change the hardware state.
+ * @PCI_DEV_FLAG_COUNT: Number of defined struct_pci_dev_flags.
+ */
+enum struct_pci_dev_flags {
+ PCI_DEV_FLAG_BUSMASTER = 0,
+
+ PCI_DEV_FLAG_COUNT
+};
+
/* struct pci_dev - describes a PCI device
*
* @supported_speeds: PCIe Supported Link Speeds Vector (+ reserved 0 at
@@ -461,7 +480,6 @@ struct pci_dev {
unsigned int pref_64_window:1; /* Pref mem window is 64-bit */
unsigned int multifunction:1; /* Multi-function device */
- unsigned int is_busmaster:1; /* Is busmaster */
unsigned int no_msi:1; /* May not use MSI */
unsigned int block_cfg_access:1; /* Config space access blocked */
unsigned int broken_parity_status:1; /* Generates false positive parity */
@@ -592,8 +610,25 @@ struct pci_dev {
u8 tph_mode; /* TPH mode */
u8 tph_req_type; /* TPH requester type */
#endif
+
+ /* PCI_DEV_FLAG_XXX flags. Use atomic bitfield operations to modify. */
+ DECLARE_BITMAP(flags, PCI_DEV_FLAG_COUNT);
};
+#define __create_pci_dev_flag_accessors(accessor_name, flag_name) \
+static inline bool pci_dev_##accessor_name(const struct pci_dev *pdev) \
+{ \
+ return test_bit(flag_name, pdev->flags); \
+} \
+static inline void pci_dev_assign_##accessor_name(struct pci_dev *pdev, bool value) \
+{ \
+ assign_bit(flag_name, pdev->flags, value); \
+}
+
+__create_pci_dev_flag_accessors(busmaster, PCI_DEV_FLAG_BUSMASTER);
+
+#undef __create_pci_dev_flag_accessors
+
static inline struct pci_dev *pci_physfn(struct pci_dev *dev)
{
#ifdef CONFIG_PCI_IOV
--
2.51.2
^ permalink raw reply related
* [PATCH 0/2] PCI: Convert bitfield flags to atomic accessors
From: Maurice Hieronymus @ 2026-07-11 15:21 UTC (permalink / raw)
To: Edward Cree, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Bjorn Helgaas, Justin Tee, Paul Ely,
James E.J. Bottomley, Martin K. Petersen, Juergen Gross,
Stefano Stabellini, Oleksandr Tyshchenko, Miguel Ojeda,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Daniel Almeida,
Tamir Duberstein, Alexandre Courbot, Onur Özkan,
Borislav Petkov, Tony Luck
Cc: Danilo Krummrich, rust-for-linux, netdev, linux-net-drivers,
linux-kernel, linux-pci, linux-scsi, xen-devel, linux-edac,
Maurice Hieronymus
`struct pci_dev` keeps ~60 flags in one C bitfield. Bits sharing a
word must not be modified concurrently, but several writers take no
common lock: `pci_set_master()` writes `is_busmaster` and can run
without the device lock (e.g. runtime PM resume paths),
`pci_disable_device()` clears it, and `broken_parity_status_store()`
writes the same word from sysfs at any time without any lock.
Convert these two bits to a new public `flags` bitmap accessed with
atomic bitops, mirroring how the driver core replaced its
`offline`/`offline_disabled` bitfield in commit a7cc262a1135 ("driver
core: Replace dev->offline + ->offline_disabled with accessors").
More bits can follow the same pattern later.
An alternative would be to reuse `priv_flags`, but its bit definitions
and accessors are deliberately private to drivers/pci, while
`is_busmaster` is accessed by xen-pciback, lpfc and sfc. Happy to
respin that way if preferred.
This is also a prerequisite for the Rust device enabling API rework
[1]: the guard object planned there calls `pci_disable_device()` from
contexts that may run concurrently with `pci_set_master()`, which
requires `is_busmaster` to not be part of a shared bitfield word.
Link: https://lore.kernel.org/rust-for-linux/DJOEYVBS17MJ.1YD3TNGQBWHNK@kernel.org/ [1]
Signed-off-by: Maurice Hieronymus <mhi@mailbox.org>
---
Maurice Hieronymus (2):
PCI: Replace pci_dev->is_busmaster with accessors
PCI: Replace pci_dev->broken_parity_status with accessors
drivers/edac/edac_pci_sysfs.c | 4 +--
drivers/net/ethernet/sfc/falcon/farch.c | 2 +-
drivers/net/ethernet/sfc/siena/farch.c | 2 +-
drivers/pci/pci-driver.c | 2 +-
drivers/pci/pci-sysfs.c | 4 +--
drivers/pci/pci.c | 6 ++---
drivers/scsi/lpfc/lpfc_init.c | 4 +--
drivers/xen/xen-pciback/conf_space_header.c | 4 +--
drivers/xen/xen-pciback/pciback_ops.c | 4 +--
include/linux/pci.h | 42 +++++++++++++++++++++++++++--
10 files changed, 56 insertions(+), 18 deletions(-)
---
base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482
change-id: 20260711-pci-dev-flags-fbbcf4ff9031
Best regards,
--
Maurice Hieronymus <mhi@mailbox.org>
^ permalink raw reply
* Re: [PATCH] virtio_net: validate device stats reply records before use
From: Michael S. Tsirkin @ 2026-07-11 15:20 UTC (permalink / raw)
To: Michael Bommarito
Cc: Jason Wang, Xuan Zhuo, Eugenio Pérez, Andrew Lunn,
Jakub Kicinski, Paolo Abeni, virtualization, netdev, linux-kernel,
stable
In-Reply-To: <20260711150754.2918392-1-michael.bommarito@gmail.com>
On Sat, Jul 11, 2026 at 11:07:54AM -0400, Michael Bommarito wrote:
> __virtnet_get_hw_stats() walks the device statistics reply buffer with
> "for (p = reply; p - reply < res_size; p += le16_to_cpu(hdr->size))",
> using each record's device-supplied hdr->size as the stride without
> checking that a full struct virtio_net_stats_reply_hdr remains, that
> hdr->size is nonzero and matches the expected size for hdr->type, or that
> the record fits within res_size. A backend that returns hdr->size == 0
> spins the loop forever; a short or oversized size drives out-of-bounds
> reads in virtnet_fill_stats().
>
> Impact: a malicious or compromised virtio-net backend hangs the CPU
> running the guest's device-statistics query in an infinite loop
> (hdr->size == 0), or drives an out-of-bounds read of the reply buffer.
> This matters most for a confidential guest, where the host is outside the
> trust boundary.
Why does it "matter most", or at all, there?
Host can always deny guest service. In fact, this is how cloud vendors
charge their clients, by denying service to whoever did not pay them.
> Validate each record before use: require a full header in the remaining
> bytes, a nonzero hdr->size that is at least the header size and matches the
> size expected for hdr->type, and that the record fits within res_size; stop
> the walk otherwise. Add virtnet_stats_reply_size() for the per-type size.
I'm all for making things easier to debug even when the device is buggy.
But I'm not inclined to add tons of hard to maintain code to
that end, and I would be worried broken hosts will come to
rely on drivers working around them.
>
> Fixes: 941168f8b40e ("virtio_net: support device stats")
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
> ---
> drivers/net/virtio_net.c | 42 ++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 40 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 3e2a5876c6c8c..9cbe40d218cc4 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -3532,6 +3532,7 @@ static int virtnet_tx_resize(struct virtnet_info *vi, struct send_queue *sq,
> return err;
> }
>
> +
> /*
> * Send command via the control virtqueue and check status. Commands
> * supported by the hypervisor, as indicated by feature bits, should
> @@ -3546,6 +3547,7 @@ static bool virtnet_send_command_reply(struct virtnet_info *vi, u8 class, u8 cmd
> bool ok;
> int ret;
>
> +
> /* Caller should know better */
> BUG_ON(!virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ));
>
we don't need this.
> @@ -4927,6 +4929,32 @@ static void virtnet_fill_stats(struct virtnet_info *vi, u32 qid,
> }
> }
>
> +static int virtnet_stats_reply_size(u8 type)
> +{
> + switch (type) {
> + case VIRTIO_NET_STATS_TYPE_REPLY_CVQ:
> + return sizeof(struct virtio_net_stats_cvq);
> + case VIRTIO_NET_STATS_TYPE_REPLY_RX_BASIC:
> + return sizeof(struct virtio_net_stats_rx_basic);
> + case VIRTIO_NET_STATS_TYPE_REPLY_RX_CSUM:
> + return sizeof(struct virtio_net_stats_rx_csum);
> + case VIRTIO_NET_STATS_TYPE_REPLY_RX_GSO:
> + return sizeof(struct virtio_net_stats_rx_gso);
> + case VIRTIO_NET_STATS_TYPE_REPLY_RX_SPEED:
> + return sizeof(struct virtio_net_stats_rx_speed);
> + case VIRTIO_NET_STATS_TYPE_REPLY_TX_BASIC:
> + return sizeof(struct virtio_net_stats_tx_basic);
> + case VIRTIO_NET_STATS_TYPE_REPLY_TX_CSUM:
> + return sizeof(struct virtio_net_stats_tx_csum);
> + case VIRTIO_NET_STATS_TYPE_REPLY_TX_GSO:
> + return sizeof(struct virtio_net_stats_tx_gso);
> + case VIRTIO_NET_STATS_TYPE_REPLY_TX_SPEED:
> + return sizeof(struct virtio_net_stats_tx_speed);
> + default:
> + return sizeof(struct virtio_net_stats_reply_hdr);
> + }
> +}
> +
> static int __virtnet_get_hw_stats(struct virtnet_info *vi,
> struct virtnet_stats_ctx *ctx,
> struct virtio_net_ctrl_queue_stats *req,
> @@ -4936,7 +4964,7 @@ static int __virtnet_get_hw_stats(struct virtnet_info *vi,
> struct scatterlist sgs_in, sgs_out;
> void *p;
> u32 qid;
> - int ok;
> + int hdr_size, ok, remaining;
>
> sg_init_one(&sgs_out, req, req_size);
> sg_init_one(&sgs_in, reply, res_size);
> @@ -4948,8 +4976,17 @@ static int __virtnet_get_hw_stats(struct virtnet_info *vi,
> if (!ok)
> return ok;
>
> - for (p = reply; p - reply < res_size; p += le16_to_cpu(hdr->size)) {
> + for (p = reply; p - reply < res_size; p += hdr_size) {
> + remaining = res_size - (p - reply);
> + if (remaining < sizeof(*hdr))
> + return -EINVAL;
> +
> hdr = p;
> + hdr_size = le16_to_cpu(hdr->size);
> + if (hdr_size < virtnet_stats_reply_size(hdr->type) ||
> + hdr_size > remaining)
> + return -EINVAL;
> +
> qid = le16_to_cpu(hdr->vq_index);
> virtnet_fill_stats(vi, qid, ctx, p, false, hdr->type);
> }
That's a lot of fragile code for unclear benefit.
> @@ -7305,3 +7342,4 @@ module_exit(virtio_net_driver_exit);
> MODULE_DEVICE_TABLE(virtio, id_table);
> MODULE_DESCRIPTION("Virtio network driver");
> MODULE_LICENSE("GPL");
> +
> --
> 2.53.0
^ permalink raw reply
* [PATCH net v5 2/2] amt: make the head writable before rewriting the L2 header
From: Michael Bommarito @ 2026-07-11 15:19 UTC (permalink / raw)
To: Taehee Yoo, Andrew Lunn, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni
Cc: netdev, linux-kernel
In-Reply-To: <20260711151934.2955226-1-michael.bommarito@gmail.com>
amt_multicast_data_handler(), amt_membership_query_handler() and
amt_update_handler() rewrite the ethernet header of the decapsulated skb
in place (eth->h_proto, eth->h_dest and, for the query, also
eth->h_source) before handing it up the stack. The skb head may be
shared, for example when a packet tap has cloned it on the underlay
interface, so writing through it corrupts the other reader's copy.
Call skb_cow_head() before the rewrite so the head is private. It is
placed before the pointers into the head are (re-)derived, so a
reallocation caused by the copy is picked up by those derivations.
Fixes: cbc21dc1cfe9 ("amt: add data plane of amt interface")
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
drivers/net/amt.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/net/amt.c b/drivers/net/amt.c
index 35e77af76bd9d..b733309b866ff 100644
--- a/drivers/net/amt.c
+++ b/drivers/net/amt.c
@@ -2320,6 +2320,9 @@ static bool amt_multicast_data_handler(struct amt_dev *amt, struct sk_buff *skb)
skb_reset_mac_header(skb);
skb_pull(skb, sizeof(*eth));
+ if (skb_cow_head(skb, 0))
+ return true;
+
if (!pskb_may_pull(skb, sizeof(*iph)))
return true;
iph = ip_hdr(skb);
@@ -2396,6 +2399,8 @@ static bool amt_membership_query_handler(struct amt_dev *amt,
skb_reset_network_header(skb);
eth = eth_hdr(skb);
ether_addr_copy(h_source, oeth->h_source);
+ if (skb_cow_head(skb, 0))
+ return true;
if (!pskb_may_pull(skb, sizeof(*iph)))
return true;
@@ -2521,6 +2526,9 @@ static bool amt_update_handler(struct amt_dev *amt, struct sk_buff *skb)
if (!pskb_may_pull(skb, sizeof(*iph)))
return true;
+ if (skb_cow_head(skb, 0))
+ return true;
+
iph = ip_hdr(skb);
if (iph->version == 4) {
if (ip_mc_check_igmp(skb)) {
--
2.53.0
^ permalink raw reply related
* [PATCH net v5 1/2] amt: re-read skb header pointers after every pull
From: Michael Bommarito @ 2026-07-11 15:19 UTC (permalink / raw)
To: Taehee Yoo, Andrew Lunn, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni
Cc: netdev, linux-kernel
In-Reply-To: <20260711151934.2955226-1-michael.bommarito@gmail.com>
Several AMT receive and transmit paths cache a pointer into the skb head
(ip_hdr(), ipv6_hdr(), eth_hdr() or the AMT message header) and then call
a helper that can reallocate that head before the cached pointer is used
again. pskb_may_pull(), ip_mc_may_pull(), ipv6_mc_may_pull(),
iptunnel_pull_header(), ip_mc_check_igmp() and ipv6_mc_check_mld() can all
free the old head and move the data, so a pointer taken before the call
dangles afterwards and the later access is a use-after-free of the freed
head.
The affected sites are:
amt_rcv() caches ip_hdr() before amt_parse_type() pulls, then reads
iph->saddr.
amt_dev_xmit() caches ip_hdr()/ipv6_hdr() before ip_mc_check_igmp()/
ipv6_mc_check_mld() and pskb_may_pull(), then reads the group address.
amt_multicast_data_handler() caches eth_hdr() before pskb_may_pull(),
then writes the L2 header.
amt_membership_query_handler() caches the AMT header, the outer and
inner eth_hdr() and ip_hdr() before iptunnel_pull_header() and several
pulls, then reads and writes them.
amt_igmpv3_report_handler() and amt_mldv2_report_handler() cache
ip_hdr()/ipv6_hdr() and the current group record and read the record
count from the report header inside the record loop, across the
*_mc_may_pull() calls.
amt_update_handler() caches ip_hdr() and the AMT membership-update
header before pskb_may_pull(), iptunnel_pull_header(),
ip_mc_check_igmp() and the report handler, then reads iph->daddr and
amtmu->nonce / amtmu->response_mac.
Fix each site by either snapshotting the scalar that is used after the
pull before the first pull runs, or re-deriving the header pointer from
the skb after the last pull that can move the head. Values that are
stable across the pull (source and group address, the response MAC and
nonce, the record count, the outer source MAC) are snapshotted; pointers
that are written through or read repeatedly are re-derived.
Fixes: cbc21dc1cfe9 ("amt: add data plane of amt interface")
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
drivers/net/amt.c | 79 +++++++++++++++++++++++++++++++++--------------
1 file changed, 55 insertions(+), 24 deletions(-)
diff --git a/drivers/net/amt.c b/drivers/net/amt.c
index 951dd10e192b7..35e77af76bd9d 100644
--- a/drivers/net/amt.c
+++ b/drivers/net/amt.c
@@ -1211,7 +1211,7 @@ static netdev_tx_t amt_dev_xmit(struct sk_buff *skb, struct net_device *dev)
data = true;
}
v6 = false;
- group.ip4 = iph->daddr;
+ group.ip4 = ip_hdr(skb)->daddr;
#if IS_ENABLED(CONFIG_IPV6)
} else if (iph->version == 6) {
ip6h = ipv6_hdr(skb);
@@ -1235,7 +1235,7 @@ static netdev_tx_t amt_dev_xmit(struct sk_buff *skb, struct net_device *dev)
data = true;
}
v6 = true;
- group.ip6 = ip6h->daddr;
+ group.ip6 = ipv6_hdr(skb)->daddr;
#endif
} else {
dev->stats.tx_errors++;
@@ -1278,12 +1278,12 @@ static netdev_tx_t amt_dev_xmit(struct sk_buff *skb, struct net_device *dev)
hlist_for_each_entry_rcu(gnode, &tunnel->groups[hash],
node) {
if (!v6) {
- if (gnode->group_addr.ip4 == iph->daddr)
+ if (gnode->group_addr.ip4 == group.ip4)
goto found;
#if IS_ENABLED(CONFIG_IPV6)
} else {
if (ipv6_addr_equal(&gnode->group_addr.ip6,
- &ip6h->daddr))
+ &group.ip6))
goto found;
#endif
}
@@ -2000,14 +2000,18 @@ static void amt_igmpv3_report_handler(struct amt_dev *amt, struct sk_buff *skb,
struct igmpv3_report *ihrv3 = igmpv3_report_hdr(skb);
int len = skb_transport_offset(skb) + sizeof(*ihrv3);
void *zero_grec = (void *)&igmpv3_zero_grec;
- struct iphdr *iph = ip_hdr(skb);
struct amt_group_node *gnode;
union amt_addr group, host;
struct igmpv3_grec *grec;
+ __be32 saddr;
u16 nsrcs;
+ u16 ngrec;
int i;
- for (i = 0; i < ntohs(ihrv3->ngrec); i++) {
+ saddr = ip_hdr(skb)->saddr;
+ ngrec = ntohs(ihrv3->ngrec);
+
+ for (i = 0; i < ngrec; i++) {
len += sizeof(*grec);
if (!ip_mc_may_pull(skb, len))
break;
@@ -2019,10 +2023,13 @@ static void amt_igmpv3_report_handler(struct amt_dev *amt, struct sk_buff *skb,
if (!ip_mc_may_pull(skb, len))
break;
+ grec = (void *)(skb->data + len - sizeof(*grec) -
+ nsrcs * sizeof(__be32));
+
memset(&group, 0, sizeof(union amt_addr));
group.ip4 = grec->grec_mca;
memset(&host, 0, sizeof(union amt_addr));
- host.ip4 = iph->saddr;
+ host.ip4 = saddr;
gnode = amt_lookup_group(tunnel, &group, &host, false);
if (!gnode) {
gnode = amt_add_group(amt, tunnel, &group, &host,
@@ -2162,14 +2169,18 @@ static void amt_mldv2_report_handler(struct amt_dev *amt, struct sk_buff *skb,
struct mld2_report *mld2r = (struct mld2_report *)icmp6_hdr(skb);
int len = skb_transport_offset(skb) + sizeof(*mld2r);
void *zero_grec = (void *)&mldv2_zero_grec;
- struct ipv6hdr *ip6h = ipv6_hdr(skb);
struct amt_group_node *gnode;
union amt_addr group, host;
struct mld2_grec *grec;
+ struct in6_addr saddr;
u16 nsrcs;
+ u16 ngrec;
int i;
- for (i = 0; i < ntohs(mld2r->mld2r_ngrec); i++) {
+ saddr = ipv6_hdr(skb)->saddr;
+ ngrec = ntohs(mld2r->mld2r_ngrec);
+
+ for (i = 0; i < ngrec; i++) {
len += sizeof(*grec);
if (!ipv6_mc_may_pull(skb, len))
break;
@@ -2181,10 +2192,13 @@ static void amt_mldv2_report_handler(struct amt_dev *amt, struct sk_buff *skb,
if (!ipv6_mc_may_pull(skb, len))
break;
+ grec = (void *)(skb->data + len - sizeof(*grec) -
+ nsrcs * sizeof(struct in6_addr));
+
memset(&group, 0, sizeof(union amt_addr));
group.ip6 = grec->grec_mca;
memset(&host, 0, sizeof(union amt_addr));
- host.ip6 = ip6h->saddr;
+ host.ip6 = saddr;
gnode = amt_lookup_group(tunnel, &group, &host, true);
if (!gnode) {
gnode = amt_add_group(amt, tunnel, &group, &host,
@@ -2305,7 +2319,6 @@ static bool amt_multicast_data_handler(struct amt_dev *amt, struct sk_buff *skb)
skb_push(skb, sizeof(*eth));
skb_reset_mac_header(skb);
skb_pull(skb, sizeof(*eth));
- eth = eth_hdr(skb);
if (!pskb_may_pull(skb, sizeof(*iph)))
return true;
@@ -2315,6 +2328,7 @@ static bool amt_multicast_data_handler(struct amt_dev *amt, struct sk_buff *skb)
if (!ipv4_is_multicast(iph->daddr))
return true;
skb->protocol = htons(ETH_P_IP);
+ eth = eth_hdr(skb);
eth->h_proto = htons(ETH_P_IP);
ip_eth_mc_map(iph->daddr, eth->h_dest);
#if IS_ENABLED(CONFIG_IPV6)
@@ -2328,6 +2342,7 @@ static bool amt_multicast_data_handler(struct amt_dev *amt, struct sk_buff *skb)
if (!ipv6_addr_is_multicast(&ip6h->daddr))
return true;
skb->protocol = htons(ETH_P_IPV6);
+ eth = eth_hdr(skb);
eth->h_proto = htons(ETH_P_IPV6);
ipv6_eth_mc_map(&ip6h->daddr, eth->h_dest);
#endif
@@ -2351,10 +2366,12 @@ static bool amt_membership_query_handler(struct amt_dev *amt,
struct sk_buff *skb)
{
struct amt_header_membership_query *amtmq;
- struct igmpv3_query *ihv3;
struct ethhdr *eth, *oeth;
+ struct igmpv3_query *ihv3;
+ u8 h_source[ETH_ALEN];
struct iphdr *iph;
int hdr_size, len;
+ u64 response_mac;
hdr_size = sizeof(*amtmq) + sizeof(struct udphdr);
if (!pskb_may_pull(skb, hdr_size))
@@ -2367,6 +2384,8 @@ static bool amt_membership_query_handler(struct amt_dev *amt,
if (amtmq->nonce != amt->nonce)
return true;
+ response_mac = amtmq->response_mac;
+
hdr_size -= sizeof(*eth);
if (iptunnel_pull_header(skb, hdr_size, htons(ETH_P_TEB), false))
return true;
@@ -2376,6 +2395,7 @@ static bool amt_membership_query_handler(struct amt_dev *amt,
skb_pull(skb, sizeof(*eth));
skb_reset_network_header(skb);
eth = eth_hdr(skb);
+ ether_addr_copy(h_source, oeth->h_source);
if (!pskb_may_pull(skb, sizeof(*iph)))
return true;
@@ -2388,6 +2408,7 @@ static bool amt_membership_query_handler(struct amt_dev *amt,
sizeof(*ihv3)))
return true;
+ iph = ip_hdr(skb);
if (!ipv4_is_multicast(iph->daddr))
return true;
@@ -2395,10 +2416,11 @@ static bool amt_membership_query_handler(struct amt_dev *amt,
skb_reset_transport_header(skb);
skb_push(skb, sizeof(*iph) + AMT_IPHDR_OPTS);
WRITE_ONCE(amt->ready4, true);
- amt->mac = amtmq->response_mac;
+ amt->mac = response_mac;
amt->req_cnt = 0;
amt->qi = ihv3->qqic;
skb->protocol = htons(ETH_P_IP);
+ eth = eth_hdr(skb);
eth->h_proto = htons(ETH_P_IP);
ip_eth_mc_map(iph->daddr, eth->h_dest);
#if IS_ENABLED(CONFIG_IPV6)
@@ -2421,10 +2443,11 @@ static bool amt_membership_query_handler(struct amt_dev *amt,
skb_reset_transport_header(skb);
skb_push(skb, sizeof(*ip6h) + AMT_IP6HDR_OPTS);
WRITE_ONCE(amt->ready6, true);
- amt->mac = amtmq->response_mac;
+ amt->mac = response_mac;
amt->req_cnt = 0;
amt->qi = mld2q->mld2q_qqic;
skb->protocol = htons(ETH_P_IPV6);
+ eth = eth_hdr(skb);
eth->h_proto = htons(ETH_P_IPV6);
ipv6_eth_mc_map(&ip6h->daddr, eth->h_dest);
#endif
@@ -2432,7 +2455,7 @@ static bool amt_membership_query_handler(struct amt_dev *amt,
return true;
}
- ether_addr_copy(eth->h_source, oeth->h_source);
+ ether_addr_copy(eth->h_source, h_source);
skb->pkt_type = PACKET_MULTICAST;
skb->ip_summed = CHECKSUM_NONE;
len = skb->len;
@@ -2455,8 +2478,11 @@ static bool amt_update_handler(struct amt_dev *amt, struct sk_buff *skb)
struct ethhdr *eth;
struct iphdr *iph;
int len, hdr_size;
+ u64 response_mac;
+ __be32 saddr;
+ __be32 nonce;
- iph = ip_hdr(skb);
+ saddr = ip_hdr(skb)->saddr;
hdr_size = sizeof(*amtmu) + sizeof(struct udphdr);
if (!pskb_may_pull(skb, hdr_size))
@@ -2466,15 +2492,18 @@ static bool amt_update_handler(struct amt_dev *amt, struct sk_buff *skb)
if (amtmu->reserved || amtmu->version)
return true;
+ nonce = amtmu->nonce;
+ response_mac = amtmu->response_mac;
+
if (iptunnel_pull_header(skb, hdr_size, skb->protocol, false))
return true;
skb_reset_network_header(skb);
list_for_each_entry_rcu(tunnel, &amt->tunnel_list, list) {
- if (tunnel->ip4 == iph->saddr) {
- if ((amtmu->nonce == tunnel->nonce &&
- amtmu->response_mac == tunnel->mac)) {
+ if (tunnel->ip4 == saddr) {
+ if ((nonce == tunnel->nonce &&
+ response_mac == tunnel->mac)) {
mod_delayed_work(amt_wq, &tunnel->gc_wq,
msecs_to_jiffies(amt_gmi(amt))
* 3);
@@ -2508,6 +2537,7 @@ static bool amt_update_handler(struct amt_dev *amt, struct sk_buff *skb)
eth = eth_hdr(skb);
skb->protocol = htons(ETH_P_IP);
eth->h_proto = htons(ETH_P_IP);
+ iph = ip_hdr(skb);
ip_eth_mc_map(iph->daddr, eth->h_dest);
#if IS_ENABLED(CONFIG_IPV6)
} else if (iph->version == 6) {
@@ -2527,6 +2557,7 @@ static bool amt_update_handler(struct amt_dev *amt, struct sk_buff *skb)
eth = eth_hdr(skb);
skb->protocol = htons(ETH_P_IPV6);
eth->h_proto = htons(ETH_P_IPV6);
+ ip6h = ipv6_hdr(skb);
ipv6_eth_mc_map(&ip6h->daddr, eth->h_dest);
#endif
} else {
@@ -2772,7 +2803,7 @@ static void amt_gw_rcv(struct amt_dev *amt, struct sk_buff *skb)
static int amt_rcv(struct sock *sk, struct sk_buff *skb)
{
struct amt_dev *amt;
- struct iphdr *iph;
+ __be32 saddr;
int type;
bool err;
@@ -2785,7 +2816,7 @@ static int amt_rcv(struct sock *sk, struct sk_buff *skb)
}
skb->dev = amt->dev;
- iph = ip_hdr(skb);
+ saddr = ip_hdr(skb)->saddr;
type = amt_parse_type(skb);
if (type == -1) {
err = true;
@@ -2795,7 +2826,7 @@ static int amt_rcv(struct sock *sk, struct sk_buff *skb)
if (amt->mode == AMT_MODE_GATEWAY) {
switch (type) {
case AMT_MSG_ADVERTISEMENT:
- if (iph->saddr != amt->discovery_ip) {
+ if (saddr != amt->discovery_ip) {
netdev_dbg(amt->dev, "Invalid Relay IP\n");
err = true;
goto drop;
@@ -2807,7 +2838,7 @@ static int amt_rcv(struct sock *sk, struct sk_buff *skb)
}
goto out;
case AMT_MSG_MULTICAST_DATA:
- if (iph->saddr != amt->remote_ip) {
+ if (saddr != amt->remote_ip) {
netdev_dbg(amt->dev, "Invalid Relay IP\n");
err = true;
goto drop;
@@ -2818,7 +2849,7 @@ static int amt_rcv(struct sock *sk, struct sk_buff *skb)
else
goto out;
case AMT_MSG_MEMBERSHIP_QUERY:
- if (iph->saddr != amt->remote_ip) {
+ if (saddr != amt->remote_ip) {
netdev_dbg(amt->dev, "Invalid Relay IP\n");
err = true;
goto drop;
base-commit: 2c7c88a412aa6d09cd04b414211b4ef8553b5309
--
2.53.0
^ permalink raw reply related
* [PATCH net v5 0/2] amt: fix use-after-free of the skb head across pulls
From: Michael Bommarito @ 2026-07-11 15:19 UTC (permalink / raw)
To: Taehee Yoo, Andrew Lunn, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni
Cc: netdev, linux-kernel
Several AMT receive and transmit paths cache a pointer into the skb head
and then call a helper that can reallocate that head before the cached
pointer is used again, so the later access reads or writes freed memory.
Patch 1 walks every AMT path and, for each pointer used after a
reallocating call, either snapshots the value before the first pull or
re-derives the pointer after the last one.
Patch 2 is a smaller, separable hardening change: the three handlers
that rewrite the ethernet header do so in place without making the head
private, which corrupts a cloned skb (for example one held by a packet
tap). It adds skb_cow_head() before the rewrite, split out so the
use-after-free fix is not held up by discussion of the clone case.
Both patches build cleanly (x86_64, CONFIG_AMT, W=1) and are
checkpatch --strict clean.
Changes since v4:
- amt_update_handler(): also snapshot amtmu->nonce and
amtmu->response_mac before iptunnel_pull_header(), which can
reallocate the head for a GSO cloned skb; the tunnel-match loop read
both fields through the stale amtmu. This is the same class as the
query handler's response_mac snapshot and was the one remaining site
the v4 fix missed.
- Remove the explanatory comments added in v4; the reason for each
snapshot/re-derive is described in the commit message instead.
- Order the local variable declarations longest-to-shortest in the
handlers that gained locals (amt_membership_query_handler and
amt_update_handler).
v4: https://lore.kernel.org/all/20260707193243.3448201-1-michael.bommarito@gmail.com/
v3: https://lore.kernel.org/all/20260626111917.802243-1-michael.bommarito@gmail.com/
v2: https://lore.kernel.org/all/20260617123443.3586930-1-michael.bommarito@gmail.com/
Michael Bommarito (2):
amt: re-read skb header pointers after every pull
amt: make the head writable before rewriting the L2 header
drivers/net/amt.c | 87 ++++++++++++++++++++++++++++++++++++++++---------------
1 file changed, 63 insertions(+), 24 deletions(-)
base-commit: 2c7c88a412aa6d09cd04b414211b4ef8553b5309
--
2.53.0
^ permalink raw reply
* Re: Ethtool : PRBS feature
From: Andrew Lunn @ 2026-07-11 15:16 UTC (permalink / raw)
To: Alexander Duyck
Cc: Lee Trager, Das, Shubham, Srinivasan, Vijay, Maxime Chevallier,
netdev@vger.kernel.org, mkubecek@suse.cz, D H, Siddaraju,
Chintalapalle, Balaji, Lindberg, Magnus,
niklas.damberg@ericsson.com, Wirandi, Jonas
In-Reply-To: <CAKgT0Ue-wMYab25gPzE8iarW2abYONv=5ZBokrLS6-DfD4WFpA@mail.gmail.com>
> Looks like they are different derivations of the polynomial for the
> given pattern. For example PRBS11 is 1 + x^9 + x^11
802.3 section "72.6.10.2.6 Training pattern" agrees with you.
, but PRBS11_0 is 1
> + x^5 + x^6 + X^10 + x^11. That is one thing to think about. When we
> say PRBS7 the assumption is we are all talking about 1 + x^6 + x^7 for
> the polynomial.
I could be missing it, but i could not quickly find PRBS7 in the
standard!
> We may want to have that clearly recorded somewhere so
> there isn't any confusion on which polynomials we are using for this
> testing as there is always a risk that somebody is playing with
> bleeding edge hardware and ends up defining a PRBS polynomial
> differently than what is expected.
I would probably give a reference to 802.3, at least for standard
tests. If a vendor has made up their own test sequences then yes,
there needs to be a clear definition of what it is.
> I almost wonder if we couldn't
> define the sequence as a bitmap instead of an enum with each bit
> representing which fields are in the sequence.
Not everything is a polynomial. Look at SSPQR, 120.5.11.2.3 SSPRQ test
pattern.
Andrew
^ permalink raw reply
* Re: [PATCH] net: qlcnic: validate unified ROM directory bounds
From: Simon Horman @ 2026-07-11 15:13 UTC (permalink / raw)
To: Pengpeng Hou
Cc: Shahed Shaikh, Manish Chopra, GR-Linux-NIC-Dev, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
netdev, linux-kernel
In-Reply-To: <20260706093601.81535-1-pengpeng@iscas.ac.cn>
On Mon, Jul 06, 2026 at 05:36:01PM +0800, Pengpeng Hou wrote:
> The unified ROM parser walks directory and data descriptor tables from the
> firmware file. The existing checks compute table and data limits with
> base + count * size or base + size before comparing with the firmware
> size. Those calculations use fields from the firmware image and can wrap
> before the comparison.
>
> Add range helpers that validate tables and entries with division and
> subtraction instead of overflowing additions. Pass the firmware size into
> the directory lookup helper, validate each directory entry before reading
> its type field, and validate bootloader, firmware and product-table
> entries before their descriptor fields are used.
>
> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
> ---
> .../net/ethernet/qlogic/qlcnic/qlcnic_init.c | 135 +++++++++++-------
> 1 file changed, 86 insertions(+), 49 deletions(-)
>
> diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_init.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_init.c
> index 9192c5ad5a16..56620aed804b 100644
> --- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_init.c
> +++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_init.c
> @@ -740,21 +740,55 @@ qlcnic_has_mn(struct qlcnic_adapter *adapter)
...
> +static struct uni_table_desc *qlcnic_get_table_desc(const u8 *unirom,
> + size_t fw_size, int section)
> {
> - u32 i, entries;
> struct uni_table_desc *directory = (struct uni_table_desc *) &unirom[0];
> - entries = le32_to_cpu(directory->num_entries);
> + u32 entries = le32_to_cpu(directory->num_entries);
> + u32 entry_size = le32_to_cpu(directory->entry_size);
> + u32 findex = le32_to_cpu(directory->findex);
> + u32 i;
I realise that this isn't the existing style of this code, but where
possible please try to move the arrangement of local variable declarations
to follow reverse xmas tree order - longest line to shortest (seems trivial
in this case). If necessary declarations and split should be split (with
all declarations coming first, then a blank line (probably not necessary
here).
>
> - for (i = 0; i < entries; i++) {
> + if (entry_size < QLCNIC_UNI_DIR_ENTRY_MIN_SIZE ||
> + !qlcnic_rom_table_valid(fw_size, findex, entries, entry_size))
> + return NULL;
>
> - u32 offs = le32_to_cpu(directory->findex) +
> - i * le32_to_cpu(directory->entry_size);
> - u32 tab_type = le32_to_cpu(*((__le32 *)&unirom[offs] + 8));
> + for (i = 0; i < entries; i++) {
> + size_t offs = findex + (size_t)i * entry_size;
I might have declared i as a size_t to avoid this cast.
> + u32 tab_type = le32_to_cpu(*((__le32 *)&unirom[offs] +
> + QLCNIC_UNI_DIR_TYPE_OFF));
>
> if (tab_type == section)
> - return (struct uni_table_desc *) &unirom[offs];
> + return (struct uni_table_desc *)&unirom[offs];
It seems that the only change above is an update to white space.
Which is a cleanup separate from the intention of this patch.
Please drop such cleanups from this patch as the add noise.
> }
>
> return NULL;
...
> @@ -789,31 +822,33 @@ qlcnic_validate_bootld(struct qlcnic_adapter *adapter)
> {
> struct uni_table_desc *tab_desc;
> struct uni_data_desc *descr;
> - u32 offs, tab_size, data_size, idx;
> const u8 *unirom = adapter->fw->data;
> + size_t offs;
> + u32 data_len, data_off, entry_size, findex, idx;
> __le32 temp;
>
> temp = *((__le32 *)&unirom[adapter->file_prd_off] +
> QLCNIC_UNI_BOOTLD_IDX_OFF);
> idx = le32_to_cpu(temp);
> - tab_desc = qlcnic_get_table_desc(unirom, QLCNIC_UNI_DIR_SECT_BOOTLD);
> + tab_desc = qlcnic_get_table_desc(unirom, adapter->fw->size,
> + QLCNIC_UNI_DIR_SECT_BOOTLD);
>
> if (!tab_desc)
> return -EINVAL;
>
> - tab_size = le32_to_cpu(tab_desc->findex) +
> - le32_to_cpu(tab_desc->entry_size) * (idx + 1);
> -
> - if (adapter->fw->size < tab_size)
> + entry_size = le32_to_cpu(tab_desc->entry_size);
> + findex = le32_to_cpu(tab_desc->findex);
> + if (entry_size < sizeof(*descr) ||
> + !qlcnic_rom_entry_valid(adapter->fw->size, findex, entry_size,
> + idx))
> return -EINVAL;
>
> - offs = le32_to_cpu(tab_desc->findex) +
> - le32_to_cpu(tab_desc->entry_size) * idx;
> + offs = findex + (size_t)entry_size * idx;
> descr = (struct uni_data_desc *)&unirom[offs];
> + data_off = le32_to_cpu(descr->findex);
> + data_len = le32_to_cpu(descr->size);
>
> - data_size = le32_to_cpu(descr->findex) + le32_to_cpu(descr->size);
> -
> - if (adapter->fw->size < data_size)
> + if (!qlcnic_rom_range_valid(adapter->fw->size, data_off, data_len))
> return -EINVAL;
This is one of two issues flagged by AI-generated of this patch at
sashiko.dev that I plan to include in this email. There are other issues,
which you may wish to look into in the context of possible follow-up. But I
feel that these two warrant consideration in the context of this patch as
they are closely related to it's intent.
This is a pre-existing issue, but does this validation need to enforce a
minimum length for the bootloader section?
The firmware loader unconditionally calculates a hardcoded size for the
bootloader section and iteratively reads this amount:
qlcnic_load_firmware() {
...
size = (QLCNIC_IMAGE_START - QLCNIC_BOOTLD_START) / 8;
...
for (i = 0; i < size; i++) {
data = le64_to_cpu(ptr64[i]);
...
}
If an attacker provides a firmware image with a valid offset near the end
of the file and a small declared size, couldn't the loop in
qlcnic_load_firmware() read past the end of the adapter->fw->data buffer?
>
> return 0;
> @@ -825,29 +860,31 @@ qlcnic_validate_fw(struct qlcnic_adapter *adapter)
> struct uni_table_desc *tab_desc;
> struct uni_data_desc *descr;
> const u8 *unirom = adapter->fw->data;
> - u32 offs, tab_size, data_size, idx;
> + size_t offs;
> + u32 data_len, data_off, entry_size, findex, idx;
> __le32 temp;
>
> temp = *((__le32 *)&unirom[adapter->file_prd_off] +
> QLCNIC_UNI_FIRMWARE_IDX_OFF);
> idx = le32_to_cpu(temp);
> - tab_desc = qlcnic_get_table_desc(unirom, QLCNIC_UNI_DIR_SECT_FW);
> + tab_desc = qlcnic_get_table_desc(unirom, adapter->fw->size,
> + QLCNIC_UNI_DIR_SECT_FW);
>
> if (!tab_desc)
> return -EINVAL;
>
> - tab_size = le32_to_cpu(tab_desc->findex) +
> - le32_to_cpu(tab_desc->entry_size) * (idx + 1);
> -
> - if (adapter->fw->size < tab_size)
> + entry_size = le32_to_cpu(tab_desc->entry_size);
> + findex = le32_to_cpu(tab_desc->findex);
> + if (entry_size < sizeof(*descr) ||
> + !qlcnic_rom_entry_valid(adapter->fw->size, findex, entry_size,
> + idx))
> return -EINVAL;
>
> - offs = le32_to_cpu(tab_desc->findex) +
> - le32_to_cpu(tab_desc->entry_size) * idx;
> + offs = findex + (size_t)entry_size * idx;
> descr = (struct uni_data_desc *)&unirom[offs];
> - data_size = le32_to_cpu(descr->findex) + le32_to_cpu(descr->size);
> -
> - if (adapter->fw->size < data_size)
> + data_off = le32_to_cpu(descr->findex);
> + data_len = le32_to_cpu(descr->size);
> + if (!qlcnic_rom_range_valid(adapter->fw->size, data_off, data_len))
> return -EINVAL;
>
> return 0;
This is the other portion of the AI-generated review of this patch on
sashiko.dev that I'd like to as you to consider in the context of this patch.
This is a pre-existing issue, but does this validation need to verify that
data_len is at least 17 bytes?
In qlcnic_get_fw_version(), the version string pointer is calculated like
this:
qlcnic_get_fw_version() {
...
ver_str = fw->data + le32_to_cpu(fw_data_desc->findex) +
le32_to_cpu(fw_data_desc->size) - 17;
...
}
If the firmware image specifies a size less than 17, won't
le32_to_cpu(fw_data_desc->size) - 17 wrap around due to 32-bit unsigned
arithmetic, leading to an out-of-bounds pointer well outside the fw->data
buffer?
...
--
pw-bot: changes-requested
^ permalink raw reply
* [PATCH] virtio_net: validate device stats reply records before use
From: Michael Bommarito @ 2026-07-11 15:07 UTC (permalink / raw)
To: Michael S . Tsirkin, Jason Wang, Xuan Zhuo, Eugenio Pérez
Cc: Andrew Lunn, Jakub Kicinski, Paolo Abeni, virtualization, netdev,
linux-kernel, stable
__virtnet_get_hw_stats() walks the device statistics reply buffer with
"for (p = reply; p - reply < res_size; p += le16_to_cpu(hdr->size))",
using each record's device-supplied hdr->size as the stride without
checking that a full struct virtio_net_stats_reply_hdr remains, that
hdr->size is nonzero and matches the expected size for hdr->type, or that
the record fits within res_size. A backend that returns hdr->size == 0
spins the loop forever; a short or oversized size drives out-of-bounds
reads in virtnet_fill_stats().
Impact: a malicious or compromised virtio-net backend hangs the CPU
running the guest's device-statistics query in an infinite loop
(hdr->size == 0), or drives an out-of-bounds read of the reply buffer.
This matters most for a confidential guest, where the host is outside the
trust boundary.
Validate each record before use: require a full header in the remaining
bytes, a nonzero hdr->size that is at least the header size and matches the
size expected for hdr->type, and that the record fits within res_size; stop
the walk otherwise. Add virtnet_stats_reply_size() for the per-type size.
Fixes: 941168f8b40e ("virtio_net: support device stats")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
drivers/net/virtio_net.c | 42 ++++++++++++++++++++++++++++++++++++++--
1 file changed, 40 insertions(+), 2 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 3e2a5876c6c8c..9cbe40d218cc4 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -3532,6 +3532,7 @@ static int virtnet_tx_resize(struct virtnet_info *vi, struct send_queue *sq,
return err;
}
+
/*
* Send command via the control virtqueue and check status. Commands
* supported by the hypervisor, as indicated by feature bits, should
@@ -3546,6 +3547,7 @@ static bool virtnet_send_command_reply(struct virtnet_info *vi, u8 class, u8 cmd
bool ok;
int ret;
+
/* Caller should know better */
BUG_ON(!virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ));
@@ -4927,6 +4929,32 @@ static void virtnet_fill_stats(struct virtnet_info *vi, u32 qid,
}
}
+static int virtnet_stats_reply_size(u8 type)
+{
+ switch (type) {
+ case VIRTIO_NET_STATS_TYPE_REPLY_CVQ:
+ return sizeof(struct virtio_net_stats_cvq);
+ case VIRTIO_NET_STATS_TYPE_REPLY_RX_BASIC:
+ return sizeof(struct virtio_net_stats_rx_basic);
+ case VIRTIO_NET_STATS_TYPE_REPLY_RX_CSUM:
+ return sizeof(struct virtio_net_stats_rx_csum);
+ case VIRTIO_NET_STATS_TYPE_REPLY_RX_GSO:
+ return sizeof(struct virtio_net_stats_rx_gso);
+ case VIRTIO_NET_STATS_TYPE_REPLY_RX_SPEED:
+ return sizeof(struct virtio_net_stats_rx_speed);
+ case VIRTIO_NET_STATS_TYPE_REPLY_TX_BASIC:
+ return sizeof(struct virtio_net_stats_tx_basic);
+ case VIRTIO_NET_STATS_TYPE_REPLY_TX_CSUM:
+ return sizeof(struct virtio_net_stats_tx_csum);
+ case VIRTIO_NET_STATS_TYPE_REPLY_TX_GSO:
+ return sizeof(struct virtio_net_stats_tx_gso);
+ case VIRTIO_NET_STATS_TYPE_REPLY_TX_SPEED:
+ return sizeof(struct virtio_net_stats_tx_speed);
+ default:
+ return sizeof(struct virtio_net_stats_reply_hdr);
+ }
+}
+
static int __virtnet_get_hw_stats(struct virtnet_info *vi,
struct virtnet_stats_ctx *ctx,
struct virtio_net_ctrl_queue_stats *req,
@@ -4936,7 +4964,7 @@ static int __virtnet_get_hw_stats(struct virtnet_info *vi,
struct scatterlist sgs_in, sgs_out;
void *p;
u32 qid;
- int ok;
+ int hdr_size, ok, remaining;
sg_init_one(&sgs_out, req, req_size);
sg_init_one(&sgs_in, reply, res_size);
@@ -4948,8 +4976,17 @@ static int __virtnet_get_hw_stats(struct virtnet_info *vi,
if (!ok)
return ok;
- for (p = reply; p - reply < res_size; p += le16_to_cpu(hdr->size)) {
+ for (p = reply; p - reply < res_size; p += hdr_size) {
+ remaining = res_size - (p - reply);
+ if (remaining < sizeof(*hdr))
+ return -EINVAL;
+
hdr = p;
+ hdr_size = le16_to_cpu(hdr->size);
+ if (hdr_size < virtnet_stats_reply_size(hdr->type) ||
+ hdr_size > remaining)
+ return -EINVAL;
+
qid = le16_to_cpu(hdr->vq_index);
virtnet_fill_stats(vi, qid, ctx, p, false, hdr->type);
}
@@ -7305,3 +7342,4 @@ module_exit(virtio_net_driver_exit);
MODULE_DEVICE_TABLE(virtio, id_table);
MODULE_DESCRIPTION("Virtio network driver");
MODULE_LICENSE("GPL");
+
--
2.53.0
^ permalink raw reply related
* [PATCH net] ila: reload IPv6 header after pskb_may_pull in checksum adjust
From: Michael Bommarito @ 2026-07-11 15:06 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, Eric Dumazet, Paolo Abeni
Cc: Simon Horman, netdev, linux-kernel, stable
ila_csum_adjust_transport() caches ip6h = ipv6_hdr(skb) before calling
pskb_may_pull(). On a non-linear skb whose transport header sits in a page
fragment, pskb_may_pull() can call __pskb_pull_tail() / pskb_expand_head()
and free the old skb head, leaving ip6h dangling; the following
get_csum_diff(ip6h, p) then reads freed memory. ila_update_ipv6_locator()
has the same pattern and additionally writes the new locator through the
stale destination-address pointer.
Impact: a remote IPv6 packet routed through a configured ILA
csum-adjust-transport route or receive-side mapping triggers a
slab-use-after-free in ila_update_ipv6_locator() (KASAN). The route or
mapping requires CAP_NET_ADMIN to configure, but trigger packets are
unauthenticated once it exists.
Reload ip6h (and the derived iaddr) after each pskb_may_pull() before use,
matching the transport-header reload the code already performs.
Fixes: 33f11d16142b ("ila: Create net/ipv6/ila directory")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
net/ipv6/ila/ila_common.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/net/ipv6/ila/ila_common.c b/net/ipv6/ila/ila_common.c
index e71571455c8a0..acedc5a84e4d7 100644
--- a/net/ipv6/ila/ila_common.c
+++ b/net/ipv6/ila/ila_common.c
@@ -85,6 +85,7 @@ static void ila_csum_adjust_transport(struct sk_buff *skb,
struct tcphdr *th = (struct tcphdr *)
(skb_network_header(skb) + nhoff);
+ ip6h = ipv6_hdr(skb);
diff = get_csum_diff(ip6h, p);
inet_proto_csum_replace_by_diff(&th->check, skb,
diff, true, true);
@@ -96,6 +97,7 @@ static void ila_csum_adjust_transport(struct sk_buff *skb,
(skb_network_header(skb) + nhoff);
if (uh->check || skb->ip_summed == CHECKSUM_PARTIAL) {
+ ip6h = ipv6_hdr(skb);
diff = get_csum_diff(ip6h, p);
inet_proto_csum_replace_by_diff(&uh->check, skb,
diff, true, true);
@@ -110,6 +112,7 @@ static void ila_csum_adjust_transport(struct sk_buff *skb,
struct icmp6hdr *ih = (struct icmp6hdr *)
(skb_network_header(skb) + nhoff);
+ ip6h = ipv6_hdr(skb);
diff = get_csum_diff(ip6h, p);
inet_proto_csum_replace_by_diff(&ih->icmp6_cksum, skb,
diff, true, true);
@@ -151,6 +154,9 @@ void ila_update_ipv6_locator(struct sk_buff *skb, struct ila_params *p,
break;
}
+ ip6h = ipv6_hdr(skb);
+ iaddr = ila_a2i(&ip6h->daddr);
+
/* Now change destination address */
iaddr->loc = p->locator;
}
--
2.53.0
^ permalink raw reply related
* [PATCH] net: mana: cap HWC init max message size to HW_CHANNEL_MAX_REQUEST_SIZE
From: Michael Bommarito @ 2026-07-11 15:06 UTC (permalink / raw)
To: Haiyang Zhang, Dexuan Cui, Long Li
Cc: K . Y . Srinivasan, Wei Liu, Andrew Lunn, Jakub Kicinski,
Paolo Abeni, netdev, linux-hyperv, linux-kernel, stable
mana_hwc_init_event_handler() in hw_channel.c stores device-advertised
HWC_INIT_DATA_MAX_REQUEST and HWC_INIT_DATA_MAX_RESPONSE values
without bounds checking. mana_hwc_alloc_dma_buf() later computes the
DMA buffer size as MANA_PAGE_ALIGN(q_depth * max_msg_size) in 32-bit
arithmetic. A malicious device returning a large max_msg_size causes
the product to wrap, allocating a small buffer while laying out
q_depth request slots at the unwrapped stride, placing slots outside
the allocation.
Impact: a compromised hypervisor device model or malicious MANA PCI
device can cause out-of-bounds DMA buffer writes during HWC channel
initialization. A reproducer is available on request.
Clamp both values to HW_CHANNEL_MAX_REQUEST_SIZE (4096), consistent
with the cap already applied at the channel-create callsite.
Fixes: ca9c54d2d6a5 ("net: mana: Add a driver for Microsoft Azure Network Adapter (MANA)")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
drivers/net/ethernet/microsoft/mana/hw_channel.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
index 48a9acea4ab6c..a0916b50cffce 100644
--- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
+++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
@@ -152,10 +152,14 @@ static void mana_hwc_init_event_handler(void *ctx, struct gdma_queue *q_self,
break;
case HWC_INIT_DATA_MAX_REQUEST:
+ if (val == 0 || val > HW_CHANNEL_MAX_REQUEST_SIZE)
+ val = HW_CHANNEL_MAX_REQUEST_SIZE;
hwc->hwc_init_max_req_msg_size = val;
break;
case HWC_INIT_DATA_MAX_RESPONSE:
+ if (val == 0 || val > HW_CHANNEL_MAX_REQUEST_SIZE)
+ val = HW_CHANNEL_MAX_REQUEST_SIZE;
hwc->hwc_init_max_resp_msg_size = val;
break;
--
2.53.0
^ permalink raw reply related
* [PATCH] net/sched: act_tunnel_key: Defer dst_release to RCU callback
From: Jamal Hadi Salim @ 2026-07-11 15:05 UTC (permalink / raw)
To: netdev
Cc: davem, edumazet, kuba, pabeni, horms, dcaratti, zdi-disclosures,
security, victor, jiri, Jamal Hadi Salim
Fix a race-condition use-after-free in tunnel_key_release_params().
The function releases the metadata_dst of the old params synchronously
via dst_release() while deferring the params struct free with
kfree_rcu(). A concurrent tunnel_key_act() reader on the datapath may
still hold the old params pointer (under rcu_read_lock_bh) and proceed
to call dst_clone(¶ms->tcft_enc_metadata->dst) after the writer's
dst_release has already pushed the dst's rcuref to RCUREF_DEAD.
zdi-disclosures@trendmicro.com produced a poc which i (and Victor) verified
that KASAN reports:
==================================================================
BUG: KASAN: slab-use-after-free in instrument_atomic_read_write include/linux/instrumented.h:112
BUG: KASAN: slab-use-after-free in atomic_sub_return_release include/linux/atomic/atomic-instrumented.h:326
BUG: KASAN: slab-use-after-free in __rcuref_put include/linux/rcuref.h:109
BUG: KASAN: slab-use-after-free in rcuref_put include/linux/rcuref.h:173
BUG: KASAN: slab-use-after-free in dst_release+0x5b/0x370 net/core/dst.c:168
Write of size 4 at addr ffff88806158de40 by task poc/9388
CPU: 0 UID: 0 PID: 9388 Comm: poc Tainted: G W 7.1.0-rc7 #7 PREEMPT(lazy)
Tainted: [W]=WARN
Hardware name: QEMU Ubuntu 25.10 PC v2 (i440FX + PIIX, + 10.1 machine, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:94
dump_stack_lvl+0x100/0x190 lib/dump_stack.c:120
print_address_description mm/kasan/report.c:378
print_report+0x139/0x4ad mm/kasan/report.c:482
kasan_report+0xe4/0x1d0 mm/kasan/report.c:595
check_region_inline mm/kasan/generic.c:186
kasan_check_range+0x125/0x200 mm/kasan/generic.c:200
instrument_atomic_read_write include/linux/instrumented.h:112
atomic_sub_return_release include/linux/atomic/atomic-instrumented.h:326
__rcuref_put include/linux/rcuref.h:109
rcuref_put include/linux/rcuref.h:173
dst_release+0x5b/0x370 net/core/dst.c:168
refdst_drop include/net/dst.h:272
skb_dst_drop include/net/dst.h:284
skb_release_head_state+0x293/0x400 net/core/skbuff.c:1163
skb_release_all net/core/skbuff.c:1187
[..]
Allocated by task 9391:
kasan_save_stack+0x30/0x50 mm/kasan/common.c:57
kasan_save_track+0x14/0x30 mm/kasan/common.c:78
poison_kmalloc_redzone mm/kasan/common.c:398
__kasan_kmalloc+0x9a/0xb0 mm/kasan/common.c:415
kasan_kmalloc include/linux/kasan.h:263
__do_kmalloc_node mm/slub.c:5296
__kmalloc_noprof+0x2f1/0x830 mm/slub.c:5308
kmalloc_noprof include/linux/slab.h:954
kzalloc_noprof include/linux/slab.h:1188
offload_action_alloc+0x2f/0x130 net/core/flow_offload.c:35
tcf_action_offload_add_ex+0x1ba/0x880 net/sched/act_api.c:258
tcf_action_offload_add net/sched/act_api.c:293
tcf_action_init+0x66e/0xa20 net/sched/act_api.c:1547
tcf_action_add+0xf6/0x5d0 net/sched/act_api.c:2101
[..]
Freed by task 9391:
kasan_save_stack+0x30/0x50 mm/kasan/common.c:57
kasan_save_track+0x14/0x30 mm/kasan/common.c:78
kasan_save_free_info+0x3b/0x70 mm/kasan/generic.c:584
poison_slab_object mm/kasan/common.c:253
__kasan_slab_free+0x6b/0x90 mm/kasan/common.c:285
kasan_slab_free include/linux/kasan.h:235
slab_free_hook mm/slub.c:2689
slab_free mm/slub.c:6251
kfree+0x21f/0x6b0 mm/slub.c:6566
tcf_action_offload_add_ex+0x4ad/0x880 net/sched/act_api.c:284
tcf_action_offload_add net/sched/act_api.c:293
tcf_action_init+0x66e/0xa20 net/sched/act_api.c:1547
tcf_action_add+0xf6/0x5d0 net/sched/act_api.c:2101
The buggy address belongs to the object at ffff88806158de00
which belongs to the cache kmalloc-256 of size 256
The buggy address is located 64 bytes inside of
freed 256-byte region [ffff88806158de00, ffff88806158df00)
The buggy address belongs to the physical page:
page: refcount:0 mapcount:0 mapping:0000000000000000 index:0xffff88806158d600 pfn:0x6158c
head: order:1 mapcount:0 entire_mapcount:0 nr_pages_mapped:0 pincount:0
flags: 0x4fff00000000240(workingset|head|node=1|zone=1|lastcpupid=0x7ff)
page_type: f5(slab)
raw: 04fff00000000240 ffff88801c841b40 ffffea0001856290 ffffea0001856190
raw: ffff88806158d600 0000000800100009 00000000f5000000 0000000000000000
head: 04fff00000000240 ffff88801c841b40 ffffea0001856290 ffffea0001856190
head: ffff88806158d600 0000000800100009 00000000f5000000 0000000000000000
head: 04fff00000000001 ffffffffffffff81 00000000ffffffff 00000000ffffffff
head: ffffffffffffffff 0000000000000000 00000000ffffffff 0000000000000002
page dumped because: kasan: bad access detected
page_owner tracks the page as allocated
page last allocated via order 1, migratetype Unmovable, gfp_mask 0xd2820(GFP_ATOMIC|__GFP_NOWARN|__GFP_NORETRY|__GFP_COMP|__GFP_NOMEMALLOC), pid 9391, tgid 9378 (poc), ts 123227323196, free_ts 0
set_page_owner include/linux/page_owner.h:32
post_alloc_hook+0xfe/0x140 mm/page_alloc.c:1853
prep_new_page mm/page_alloc.c:1861
get_page_from_freelist+0x110c/0x2fc0 mm/page_alloc.c:3941
__alloc_frozen_pages_noprof+0x263/0x2bc0 mm/page_alloc.c:5221
alloc_slab_page mm/slub.c:3278
allocate_slab mm/slub.c:3467
new_slab+0xa6/0x690 mm/slub.c:3525
refill_objects+0x271/0x420 mm/slub.c:7272
refill_sheaf mm/slub.c:2816
__pcs_replace_empty_main+0x373/0x630 mm/slub.c:4652
alloc_from_pcs mm/slub.c:4750
slab_alloc_node mm/slub.c:4884
__do_kmalloc_node mm/slub.c:5295
__kmalloc_noprof+0x66d/0x830 mm/slub.c:5308
kmalloc_noprof include/linux/slab.h:954
metadata_dst_alloc+0x26/0x90 net/core/dst.c:298
tun_rx_dst include/net/dst_metadata.h:144
__ip_tun_set_dst include/net/dst_metadata.h:208
tunnel_key_init+0xb01/0x1b90 net/sched/act_tunnel_key.c:451
tcf_action_init_1+0x46b/0x6c0 net/sched/act_api.c:1428
tcf_action_init+0x448/0xa20 net/sched/act_api.c:1503
tcf_action_add+0xf6/0x5d0 net/sched/act_api.c:2101
[..]
==================================================================
Fix by moving dst_release() into a custom RCU callback that runs
after the grace period, matching the lifetime of the containing
params struct. Readers in the datapath therefore always find a live
rcuref when calling dst_clone().
Fixes: 9174c3df1cd18 ("net/sched: act_tunnel_key: fix memory leak in case of action replace")
Reported-by: zdi-disclosures@trendmicro.com
Tested-by: Victor Nogueira <victor@mojatatu.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
net/sched/act_tunnel_key.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/net/sched/act_tunnel_key.c b/net/sched/act_tunnel_key.c
index 876b30c5709e..b14807761d82 100644
--- a/net/sched/act_tunnel_key.c
+++ b/net/sched/act_tunnel_key.c
@@ -342,14 +342,20 @@ static const struct nla_policy tunnel_key_policy[TCA_TUNNEL_KEY_MAX + 1] = {
[TCA_TUNNEL_KEY_ENC_TTL] = { .type = NLA_U8 },
};
-static void tunnel_key_release_params(struct tcf_tunnel_key_params *p)
+static void tunnel_key_release_params_rcu(struct rcu_head *head)
{
- if (!p)
- return;
+ struct tcf_tunnel_key_params *p = container_of(head, typeof(*p), rcu);
+
if (p->tcft_action == TCA_TUNNEL_KEY_ACT_SET)
dst_release(&p->tcft_enc_metadata->dst);
+ kfree(p);
+}
- kfree_rcu(p, rcu);
+static void tunnel_key_release_params(struct tcf_tunnel_key_params *p)
+{
+ if (!p)
+ return;
+ call_rcu(&p->rcu, tunnel_key_release_params_rcu);
}
static int tunnel_key_init(struct net *net, struct nlattr *nla,
--
2.43.0
^ permalink raw reply related
* Re: [PATCH bpf] bpf: tcp: Fix use-after-free in bpf_iter_tcp_established_batch()
From: Jose Fernandez (Anthropic) @ 2026-07-11 15:00 UTC (permalink / raw)
To: Kuniyuki Iwashima
Cc: Eric Dumazet, Neal Cardwell, David S. Miller, Jakub Kicinski,
Paolo Abeni, Simon Horman, Andrii Nakryiko, Yonghong Song,
Martin KaFai Lau, netdev, linux-kernel, bpf, Ben Cressey,
Daniel Borkmann
In-Reply-To: <CAAVpQUDiLwSYwFStRGpMQkGbm3j8_KJm9LVj0ykV7V3nYQ4YTg@mail.gmail.com>
Hi Kuniyuki,
Thanks for the review!
On Sat, Jul 11, 2026 at 05:36:05AM -0700, Kuniyuki Iwashima wrote:
> > + } else if (!*start_sk) {
> > + /* Remember where we left off. */
> > + *start_sk = sk;
> > }
> > + expected++;
>
> This should be incremented just after seq_sk_match()
> (see below)
Will do.
> > @@ -3167,6 +3168,10 @@ static struct sock *bpf_iter_tcp_batch(struct seq_file *seq)
> > WARN_ON_ONCE(iter->end_sk != expected);
>
> Let's say the batch array was smaller than the hash chain length
> and we reallocate the array based on "expected" w/ the bucket lock.
>
> What happens if refcount_set(..., 3) is called during reallocation ?
> bpf_iter_fill_batch() will see the larger "expected", and WARN_ON_ONCE()
> will be triggered.
Right. Moving expected++ up fixes the sizing. Since end_sk == expected
then no longer holds when a socket is skipped, I'll make the
batch-complete check and the WARN look for a leftover socket instead.
I'll send a v2 early next week.
Thanks,
Jose
^ permalink raw reply
* Re: [PATCH net v4 0/2] amt: fix use-after-free of the skb head across pulls
From: Taehee Yoo @ 2026-07-11 14:48 UTC (permalink / raw)
To: Michael Bommarito
Cc: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, netdev, linux-kernel
In-Reply-To: <20260707193243.3448201-1-michael.bommarito@gmail.com>
On Wed, Jul 8, 2026 at 4:32 AM Michael Bommarito
<michael.bommarito@gmail.com> wrote:
>
> Several AMT receive and transmit paths cache a pointer into the skb head
> and then call a helper that can reallocate that head before the cached
> pointer is used again, so the later access reads or writes freed memory.
>
> v3 addressed only the source-address reads in a subset of the handlers
> and described amt_membership_query_handler() and
> amt_multicast_data_handler() as unaffected. As the review pointed out,
> that was incomplete: those handlers keep stale eth_hdr() and AMT-header
> pointers across later pulls, the record loops in the IGMPv3 and MLDv2
> report handlers read the record count and the group record across the
> *_mc_may_pull() calls, and amt_update_handler() and amt_dev_xmit() read
> the destination address after further pulls.
>
> Patch 1 walks every AMT path and, for each pointer used after a
> reallocating call, either snapshots the value before the first pull or
> re-derives the pointer after the last one. This uses the re-derive
> approach rather than the per-value snapshot of v3, because the write
> sites cannot be expressed as a snapshot and re-derivation is already the
> idiom used elsewhere in the file.
>
> Patch 2 is a smaller, separable hardening change: the three handlers
> that rewrite the ethernet header do so in place without making the head
> private, which corrupts a cloned skb (for example one held by a packet
> tap). It adds skb_cow_head() before the rewrite, split out so the
> use-after-free fix is not held up by discussion of the clone case.
>
> Both patches build cleanly (x86_64, CONFIG_AMT=m, W=1) and are
> checkpatch --strict clean.
>
> Changes since v3:
> - Rework from the per-value source-address snapshot to re-deriving the
> header pointers after the last reallocating pull, and cover every
> affected handler (amt_dev_xmit, amt_multicast_data_handler,
> amt_membership_query_handler, the IGMPv3 and MLDv2 record loops, and
> the remaining reads in amt_update_handler), not just the
> source-address reads.
> - Correct the v3 commit-message claim that the query and multicast-data
> handlers were unaffected.
> - Add patch 2 (skb_cow_head() before the L2 rewrite).
> - Drop the v2 Acked-by from Taehee Yoo: this series is materially larger
> than what was acked.
>
> v3: https://lore.kernel.org/all/20260626111917.802243-1-michael.bommarito@gmail.com/
> v2: https://lore.kernel.org/all/20260617123443.3586930-1-michael.bommarito@gmail.com/
>
Hi Michael,
Thanks a lot for this work!
Sashiko flagged one more issue, please take a look:
https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260707193243.3448201-1-michael.bommarito%40gmail.com
Also, please remove the unnecessary comments in the patch.
AI-generated code tends to include too many comments.
One more thing: please follow the reverse Christmas tree order
(longest to shortest) for local variable declarations.
Thanks,
Taehee
> Michael Bommarito (2):
> amt: re-read skb header pointers after every pull
> amt: make the head writable before rewriting the L2 header
>
> drivers/net/amt.c | 117 +++++++++++++++++++++++++++++++++++++---------
> 1 file changed, 96 insertions(+), 21 deletions(-)
>
>
> base-commit: 5200f5f493f79f14bbdc349e402a40dfb32f23c8
> --
> 2.53.0
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox