* [PATCH iwl-next v1 2/4] ice: dpll: Use switch statements to handle pin states
From: Sergey Temerkhanov @ 2026-07-16 9:49 UTC (permalink / raw)
To: intel-wired-lan; +Cc: netdev
In-Reply-To: <20260716094912.1210865-1-sergey.temerkhanov@intel.com>
Use switch statements to handle pin states to make the code more
readable. This also makes this code more future-proof, should any
new states appear.
This also changes how direction-mismatched state requests are handled in
ice_dpll_sma_pin_state_set(). Previously, requesting CONNECTED on an
INPUT-direction SMA pin or SELECTABLE on an OUTPUT-direction pin would
fall through to ice_dpll_pin_disable() and return success. After this
change those requests return -EINVAL without issuing a firmware command.
Because the DPLL netlink core does not filter pin states by direction
before calling the driver, this is a user-visible netlink API change.
Signed-off-by: Sergey Temerkhanov <sergey.temerkhanov@intel.com>
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Reviewed-by: Przemyslaw Korba <przemyslaw.korba@intel.com>
---
drivers/net/ethernet/intel/ice/ice_dpll.c | 65 ++++++++++++++++++-----
1 file changed, 53 insertions(+), 12 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.c b/drivers/net/ethernet/intel/ice/ice_dpll.c
index fed7c9fea953..54958e17713b 100644
--- a/drivers/net/ethernet/intel/ice/ice_dpll.c
+++ b/drivers/net/ethernet/intel/ice/ice_dpll.c
@@ -1320,10 +1320,12 @@ ice_dpll_ufl_pin_state_set(const struct dpll_pin *pin, void *pin_priv,
ret = -EINVAL;
switch (p->idx) {
case ICE_DPLL_PIN_SW_1_IDX:
- if (state == DPLL_PIN_STATE_CONNECTED) {
+ switch (state) {
+ case DPLL_PIN_STATE_CONNECTED:
data &= ~ICE_SMA1_MASK;
enable = true;
- } else if (state == DPLL_PIN_STATE_DISCONNECTED) {
+ break;
+ case DPLL_PIN_STATE_DISCONNECTED:
/* Skip if U.FL1 is not active, setting TX_EN
* while DIR_EN is set would also deactivate
* the paired SMA1 output.
@@ -1334,18 +1336,21 @@ ice_dpll_ufl_pin_state_set(const struct dpll_pin *pin, void *pin_priv,
}
data |= ICE_SMA1_TX_EN;
enable = false;
- } else {
+ break;
+ default:
goto unlock;
}
target = p->output;
type = ICE_DPLL_PIN_TYPE_OUTPUT;
break;
case ICE_DPLL_PIN_SW_2_IDX:
- if (state == DPLL_PIN_STATE_SELECTABLE) {
+ switch (state) {
+ case DPLL_PIN_STATE_SELECTABLE:
data |= ICE_SMA2_DIR_EN;
data &= ~ICE_SMA2_UFL2_RX_DIS;
enable = true;
- } else if (state == DPLL_PIN_STATE_DISCONNECTED) {
+ break;
+ case DPLL_PIN_STATE_DISCONNECTED:
/* Skip if U.FL2 is not active, setting
* UFL2_RX_DIS could also disable the paired
* SMA2 input.
@@ -1357,7 +1362,8 @@ ice_dpll_ufl_pin_state_set(const struct dpll_pin *pin, void *pin_priv,
}
data |= ICE_SMA2_UFL2_RX_DIS;
enable = false;
- } else {
+ break;
+ default:
goto unlock;
}
target = p->input;
@@ -1484,14 +1490,43 @@ ice_dpll_sma_pin_state_set(const struct dpll_pin *pin, void *pin_priv,
if (ret)
goto unlock;
}
- if (sma->direction == DPLL_PIN_DIRECTION_INPUT) {
- enable = state == DPLL_PIN_STATE_SELECTABLE;
+ switch (state) {
+ case DPLL_PIN_STATE_SELECTABLE:
+ if (sma->direction == DPLL_PIN_DIRECTION_OUTPUT) {
+ enable = false;
+ ret = -EINVAL;
+ goto unlock;
+ }
+ enable = true;
+ break;
+ case DPLL_PIN_STATE_CONNECTED:
+ if (sma->direction == DPLL_PIN_DIRECTION_INPUT) {
+ enable = false;
+ ret = -EINVAL;
+ goto unlock;
+ }
+ enable = true;
+ break;
+ case DPLL_PIN_STATE_DISCONNECTED:
+ enable = false;
+ break;
+ default:
+ ret = -EINVAL;
+ goto unlock;
+ }
+
+ switch (sma->direction) {
+ case DPLL_PIN_DIRECTION_INPUT:
target = sma->input;
type = ICE_DPLL_PIN_TYPE_INPUT;
- } else {
- enable = state == DPLL_PIN_STATE_CONNECTED;
+ break;
+ case DPLL_PIN_DIRECTION_OUTPUT:
target = sma->output;
type = ICE_DPLL_PIN_TYPE_OUTPUT;
+ break;
+ default:
+ ret = -EINVAL;
+ goto unlock;
}
if (enable)
@@ -4631,7 +4666,8 @@ static int ice_dpll_init_info_sw_pins(struct ice_pf *pf)
pin->prop.capabilities = caps;
pin->pf = pf;
pin->prop.board_label = ice_dpll_sw_pin_ufl[i];
- if (i == ICE_DPLL_PIN_SW_1_IDX) {
+ switch (i) {
+ case ICE_DPLL_PIN_SW_1_IDX:
pin->direction = DPLL_PIN_DIRECTION_OUTPUT;
pin_abs_idx = ICE_DPLL_PIN_SW_OUTPUT_ABS(i);
pin->prop.freq_supported =
@@ -4641,7 +4677,8 @@ static int ice_dpll_init_info_sw_pins(struct ice_pf *pf)
pin->prop.freq_supported_num = freq_supp_num;
pin->input = NULL;
pin->output = &d->outputs[pin_abs_idx];
- } else if (i == ICE_DPLL_PIN_SW_2_IDX) {
+ break;
+ case ICE_DPLL_PIN_SW_2_IDX:
pin->direction = DPLL_PIN_DIRECTION_INPUT;
pin_abs_idx = ICE_DPLL_PIN_SW_INPUT_ABS(i) +
input_idx_offset;
@@ -4654,6 +4691,10 @@ static int ice_dpll_init_info_sw_pins(struct ice_pf *pf)
pin->prop.capabilities =
(DPLL_PIN_CAPABILITIES_PRIORITY_CAN_CHANGE |
caps);
+ break;
+ default:
+ dev_err(ice_pf_to_dev(pf), "Invalid U.FL pin index: %d\n", i);
+ return -EINVAL;
}
pin->muxed = &d->sma[i];
ice_dpll_phase_range_set(&pin->prop.phase_range, phase_adj_max);
--
2.53.0
^ permalink raw reply related
* [PATCH iwl-next v1 1/4] ice: dpll: Rework multiplexed pin notifications
From: Sergey Temerkhanov @ 2026-07-16 9:49 UTC (permalink / raw)
To: intel-wired-lan; +Cc: netdev
In-Reply-To: <20260716094912.1210865-1-sergey.temerkhanov@intel.com>
Use a pointer to the struct ice_dpll_pin to link multiplexed
pins. This allows to simplify the selection logic.
Signed-off-by: Sergey Temerkhanov <sergey.temerkhanov@intel.com>
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Reviewed-by: Przemyslaw Korba <przemyslaw.korba@intel.com>
---
drivers/net/ethernet/intel/ice/ice_dpll.c | 60 +++++++++++------------
drivers/net/ethernet/intel/ice/ice_dpll.h | 1 +
2 files changed, 31 insertions(+), 30 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.c b/drivers/net/ethernet/intel/ice/ice_dpll.c
index 1ca137f67dd4..fed7c9fea953 100644
--- a/drivers/net/ethernet/intel/ice/ice_dpll.c
+++ b/drivers/net/ethernet/intel/ice/ice_dpll.c
@@ -544,6 +544,29 @@ ice_dpll_pin_disable(struct ice_hw *hw, struct ice_dpll_pin *pin,
return ret;
}
+/**
+ * ice_dpll_sw_pin_notify_peer - notify the paired SW pin after a state change
+ * @changed: the SW pin that was explicitly changed (already notified by dpll core)
+ *
+ * SMA and U.FL pins share physical signal paths in pairs (SMA1/U.FL1 and
+ * SMA2/U.FL2). When one pin's routing changes via the PCA9575 GPIO
+ * expander, the paired pin's state may also change. Send a change
+ * notification for the peer pin so userspace consumers monitoring the
+ * peer via dpll netlink learn about the update.
+ *
+ * Context: Called from dpll_pin_ops callbacks after pf->dplls.lock is
+ * released. Uses __dpll_pin_change_ntf() because dpll_lock is
+ * still held by the dpll netlink layer.
+ */
+static void ice_dpll_sw_pin_notify_peer(struct ice_dpll_pin *changed)
+{
+ struct ice_dpll_pin *peer;
+
+ peer = changed->muxed;
+ if (peer->pin)
+ __dpll_pin_change_ntf(peer->pin);
+}
+
/**
* ice_dpll_pin_store_state - updates the state of pin in SW bookkeeping
* @pin: pointer to a pin
@@ -1171,32 +1194,6 @@ ice_dpll_input_state_get(const struct dpll_pin *pin, void *pin_priv,
extack, ICE_DPLL_PIN_TYPE_INPUT);
}
-/**
- * ice_dpll_sw_pin_notify_peer - notify the paired SW pin after a state change
- * @d: pointer to dplls struct
- * @changed: the SW pin that was explicitly changed (already notified by dpll core)
- *
- * SMA and U.FL pins share physical signal paths in pairs (SMA1/U.FL1 and
- * SMA2/U.FL2). When one pin's routing changes via the PCA9575 GPIO
- * expander, the paired pin's state may also change. Send a change
- * notification for the peer pin so userspace consumers monitoring the
- * peer via dpll netlink learn about the update.
- *
- * Context: Called from dpll_pin_ops callbacks after pf->dplls.lock is
- * released. Uses __dpll_pin_change_ntf() because dpll_lock is
- * still held by the dpll netlink layer.
- */
-static void ice_dpll_sw_pin_notify_peer(struct ice_dplls *d,
- struct ice_dpll_pin *changed)
-{
- struct ice_dpll_pin *peer;
-
- peer = (changed >= d->sma && changed < d->sma + ICE_DPLL_PIN_SW_NUM) ?
- &d->ufl[changed->idx] : &d->sma[changed->idx];
- if (peer->pin)
- __dpll_pin_change_ntf(peer->pin);
-}
-
/**
* ice_dpll_sma_direction_set - set direction of SMA pin
* @p: pointer to a pin
@@ -1258,7 +1255,7 @@ static int ice_dpll_sma_direction_set(struct ice_dpll_pin *p,
* backing pin when U.FL becomes inactive because the SMA pin may
* still be using it.
*/
- peer = &d->ufl[p->idx];
+ peer = p->muxed;
if (peer->active) {
struct ice_dpll_pin *target;
enum ice_dpll_pin_type type;
@@ -1388,7 +1385,7 @@ ice_dpll_ufl_pin_state_set(const struct dpll_pin *pin, void *pin_priv,
unlock:
mutex_unlock(&pf->dplls.lock);
if (!ret)
- ice_dpll_sw_pin_notify_peer(&pf->dplls, p);
+ ice_dpll_sw_pin_notify_peer(p);
return ret;
}
@@ -1508,7 +1505,7 @@ ice_dpll_sma_pin_state_set(const struct dpll_pin *pin, void *pin_priv,
unlock:
mutex_unlock(&pf->dplls.lock);
if (!ret)
- ice_dpll_sw_pin_notify_peer(&pf->dplls, sma);
+ ice_dpll_sw_pin_notify_peer(sma);
return ret;
}
@@ -1705,7 +1702,7 @@ ice_dpll_pin_sma_direction_set(const struct dpll_pin *pin, void *pin_priv,
ret = ice_dpll_sma_direction_set(p, direction, extack);
mutex_unlock(&pf->dplls.lock);
if (!ret)
- ice_dpll_sw_pin_notify_peer(&pf->dplls, p);
+ ice_dpll_sw_pin_notify_peer(p);
return ret;
}
@@ -4623,6 +4620,8 @@ static int ice_dpll_init_info_sw_pins(struct ice_pf *pf)
if (pin->input->ref_sync)
pin->ref_sync = pin->input->ref_sync - pin_abs_idx;
pin->output = &d->outputs[ICE_DPLL_PIN_SW_OUTPUT_ABS(i)];
+ pin->muxed = &d->ufl[i];
+
ice_dpll_phase_range_set(&pin->prop.phase_range, phase_adj_max);
}
for (i = 0; i < ICE_DPLL_PIN_SW_NUM; i++) {
@@ -4656,6 +4655,7 @@ static int ice_dpll_init_info_sw_pins(struct ice_pf *pf)
(DPLL_PIN_CAPABILITIES_PRIORITY_CAN_CHANGE |
caps);
}
+ pin->muxed = &d->sma[i];
ice_dpll_phase_range_set(&pin->prop.phase_range, phase_adj_max);
}
diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.h b/drivers/net/ethernet/intel/ice/ice_dpll.h
index c59d746a8567..c102ff2649d9 100644
--- a/drivers/net/ethernet/intel/ice/ice_dpll.h
+++ b/drivers/net/ethernet/intel/ice/ice_dpll.h
@@ -78,6 +78,7 @@ struct ice_dpll_pin {
s32 phase_adjust;
struct ice_dpll_pin *input;
struct ice_dpll_pin *output;
+ struct ice_dpll_pin *muxed;
enum dpll_pin_direction direction;
s64 phase_offset;
u8 status;
--
2.53.0
^ permalink raw reply related
* [PATCH iwl-next v1 0/4] Rework and fix ice dpll pin control
From: Sergey Temerkhanov @ 2026-07-16 9:49 UTC (permalink / raw)
To: intel-wired-lan; +Cc: netdev
This series reworks and clarifies ice DPLL control logic.
Contains refactoring changes for better readability and
maintainability as well as the changes making the pin
controls compliant to design requirements.
Sergey Temerkhanov (4):
ice: dpll: Rework multiplexed pin notifications
ice: dpll: Use switch statements to handle pin states
ice: dpll: Rework U.FL muxed pin (SMA) control
ice: dpll: Rework the SMA control logic to match the requirements
drivers/net/ethernet/intel/ice/ice_dpll.c | 248 +++++++++++++++++-----
drivers/net/ethernet/intel/ice/ice_dpll.h | 1 +
2 files changed, 193 insertions(+), 56 deletions(-)
base-commit: bf696cf19d64727a5a95126733603269ed8c42c2
--
2.53.0
^ permalink raw reply
* Re: [PATCH iproute2-next 1/2] seg6: add support for lookup attribute in SRv6 encap routes
From: Nicolas Dichtel @ 2026-07-16 9:48 UTC (permalink / raw)
To: Andrea Mayer, David Ahern, netdev
Cc: Stephen Hemminger, Stefano Salsano, Ahmed Abdelsalam,
Paolo Lungaroni, Justin Iurman, Anthony Doeraene
In-Reply-To: <20260712021155.7621-2-andrea.mayer@uniroma2.it>
Le 12/07/2026 à 04:11, Andrea Mayer a écrit :
> Add support for the new optional "lookup" attribute for seg6 encap
> routes. It selects the FIB table for the post-encap SID route lookup
> and accepts a table number or a table name.
>
> Examples:
>
> # 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
>
> When the attribute is omitted, the post-encap SID route lookup behaves
> as before, using the current routing context (e.g. the tables selected
> according to the routing policy database).
>
> Signed-off-by: Andrea Mayer <andrea.mayer@uniroma2.it>
Reviewed-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
^ permalink raw reply
* Re: [PATCH net-next 2/2] selftests: seg6: add test for post-encap SID route lookup
From: Nicolas Dichtel @ 2026-07-16 9:46 UTC (permalink / raw)
To: Andrea Mayer, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni
Cc: David Ahern, Simon Horman, Shuah Khan, Justin Iurman,
Anthony Doeraene, Stefano Salsano, Ahmed Abdelsalam,
Paolo Lungaroni, netdev, linux-kselftest, linux-kernel
In-Reply-To: <20260711162907.6521-3-andrea.mayer@uniroma2.it>
Le 11/07/2026 à 18:29, Andrea Mayer a écrit :
> Add a selftest for the SEG6_IPTUNNEL_TABLE attribute, which selects the FIB
> table for the post-encap SID route lookup. This looks up the route for the
> first SID, the outer destination of the encapsulated packet.
>
> Two routers provide L3 VPN services over an IPv6 underlay. Each router uses
> a separate VRF per tenant, with default blackhole routes (IPv4 and IPv6)
> that drop unmatched traffic. Tenant traffic is encapsulated, then
> decapsulated with an End.DT46.
> The encap routes are installed in the tenant VRF, but the routes that match
> the first SIDs live in a separate underlay table (500). The "lookup 500"
> attribute points the lookup there rather than to the VRF.
>
> The test covers both the input path, where forwarded host traffic triggers
> encapsulation, and the output path, where a router originates traffic from
> its own loopback inside a VRF.
> With the "lookup" attribute, traffic reaches its destination on both paths.
> Without it, on the input path the lookup stays in the VRF and hits the
> blackhole, and on the output path it falls through to the main table, which
> has no matching route.
>
> Signed-off-by: Andrea Mayer <andrea.mayer@uniroma2.it>
Reviewed-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
^ permalink raw reply
* Re: [PATCH net-next] rndis_host: add overflow check in rndis_rx_fixup()
From: Simon Horman @ 2026-07-16 9:43 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: netdev, linux-usb, linux-kernel, Griffin Kroah-Hartman,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Shaoxu Liu
In-Reply-To: <2026070900-denim-brook-52d4@gregkh>
On Thu, Jul 09, 2026 at 02:24:01PM +0200, Greg Kroah-Hartman wrote:
> From: Griffin Kroah-Hartman <griffin@kroah.com>
>
> Add an overflow check to ensure that data_offset + data_len + 8 does not
> wrap, which would enable an OOB read of the USB data buffer.
>
> Assisted-by: gkh_clanker_1000
> Cc: Andrew Lunn <andrew+netdev@lunn.ch>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Paolo Abeni <pabeni@redhat.com>
> Cc: Shaoxu Liu <shaoxul@foxmail.com>
> Signed-off-by: Griffin Kroah-Hartman <griffin@kroah.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Simon Horman <horms@kernel.org>
^ permalink raw reply
* Re: [PATCH] vhost: reject zero-size IOTLB INVALIDATE
From: Eugenio Perez Martin @ 2026-07-16 9:41 UTC (permalink / raw)
To: Weimin Xiong; +Cc: virtualization, mst, jasowangio, netdev, kvm, xiongweimin
In-Reply-To: <20260716030236.124322-1-xiongwm2026@163.com>
On Thu, Jul 16, 2026 at 5:02 AM Weimin Xiong <xiongwm2026@163.com> wrote:
>
> From: xiongweimin <xiongweimin@kylinos.cn>
>
> Reject VHOST_IOTLB_INVALIDATE messages with size == 0 to prevent
> iova + size - 1 from underflowing to U64_MAX, which would
> incorrectly delete the entire IOTLB.
>
> Signed-off-by: xiongweimin <xiongweimin@kylinos.cn>
> ---
> drivers/vhost/vhost.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index 3c080c454e374cabd7321416ed92c5f7d3135254..xxxxxxxxxx 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -1656,6 +1656,10 @@ static int vhost_process_iotlb_msg(struct vhost_dev *dev, u32 asid,
> if (!dev->iotlb) {
> ret = -EFAULT;
> break;
> + }
> + if (!msg->size) {
> + ret = -EINVAL;
> + break;
> }
I think the issue is real, but how about adding the condition to the
caller vhost_chr_write_iter? It is already the
if (msg.type == VHOST_IOTLB_UPDATE && msg.size == 0) {
ret = -EINVAL;
goto done;
}
So it should be somthing in the line of:
if ((msg.type == VHOST_IOTLB_UPDATE || msg.type ==
VHOST_IOTLB_INVALIDATE) && msg.size == 0) {
ret = -EINVAL;
goto done;
}
With that, please add my acked-by.
> vhost_vq_meta_reset(dev);
> vhost_iotlb_del_range(dev->iotlb, msg->iova,
> --
> 2.39.3
>
^ permalink raw reply
* Re: [PATCH v4 6/7] net: wwan: t9xx: Add AT & MBIM WWAN ports
From: Simon Horman @ 2026-07-16 9:34 UTC (permalink / raw)
To: jackbb_wu
Cc: Simon Horman, loic.poulain, ryazanov.s.a, johannes, andrew+netdev,
davem, edumazet, kuba, pabeni, wen-zhi.huang, shi-wei.yeh,
Minano.tseng, matthias.bgg, angelogioacchino.delregno, corbet,
skhan, linux-kernel, netdev, linux-arm-kernel, linux-mediatek,
linux-doc
In-Reply-To: <20260709-t9xx_driver_v1-v4-6-a8c009d509c5@compal.com>
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
---
--- Patch [6]: [PATCH v4 6/7] net: wwan: t9xx: Add AT & MBIM WWAN ports ---
commit 1739a55610bf7a44ce24874819e01c07236d9442
Author: Jack Wu <jackbb_wu@compal.com>
net: wwan: t9xx: Add AT & MBIM WWAN ports
Add AT & MBIM ports to the port infrastructure.
The WWAN initialization method is responsible for creating the
corresponding ports using the WWAN framework infrastructure. The
implemented WWAN port operations are start, stop, tx, tx_blocking
and tx_poll.
> diff --git a/drivers/net/wwan/t9xx/mtk_port_io.c b/drivers/net/wwan/t9xx/mtk_port_io.c
> index e3a2de6d2f297..882254b74026f 100644
> --- a/drivers/net/wwan/t9xx/mtk_port_io.c
> +++ b/drivers/net/wwan/t9xx/mtk_port_io.c
[ ... ]
> @@ -234,6 +376,198 @@ static const struct port_ops port_internal_ops = {
> .recv = mtk_port_internal_recv,
> };
>
> +static int mtk_port_wwan_open(struct wwan_port *w_port)
[ ... ]
> +static int mtk_port_wwan_write(struct wwan_port *w_port, struct sk_buff *skb)
> +{
> + struct mtk_port *port = wwan_port_get_drvdata(w_port);
> + union user_buf user_buf;
> + int ret;
> +
> + if (unlikely(!skb->len)) {
> + consume_skb(skb);
> + return 0;
> + }
> +
> + port->info.flags &= ~PORT_F_BLOCKING;
> + user_buf.kbuf = (void *)skb->data;
> + ret = mtk_port_common_write(port, user_buf, skb->len, false);
> + if (ret < 0)
> + return ret;
[Severity: High]
If mtk_port_common_write() successfully sends the first few fragments but
fails on subsequent ones, returning a positive partial write count, does
this code silently drop the remaining data?
The condition ret < 0 is false for partial writes, so the SKB is consumed
and 0 is returned to the caller, falsely reporting complete success to the
WWAN core.
> +
> + consume_skb(skb);
> + return 0;
> +}
> +
> +static int mtk_port_wwan_write_blocking(struct wwan_port *w_port, struct sk_buff *skb)
> +{
> + struct mtk_port *port = wwan_port_get_drvdata(w_port);
> + union user_buf user_buf;
> + int ret;
> +
> + if (unlikely(!skb->len)) {
> + consume_skb(skb);
> + return 0;
> + }
> +
> + port->info.flags |= PORT_F_BLOCKING;
> + user_buf.kbuf = (void *)skb->data;
> + ret = mtk_port_common_write(port, user_buf, skb->len, false);
> + if (ret < 0)
> + return ret;
[Severity: High]
Can the same silent drop of partially written SKBs happen here?
> +
> + consume_skb(skb);
> + return 0;
> +}
[ ... ]
> +static void mtk_port_wwan_enable(struct mtk_port *port)
> +{
> + struct mtk_port_mngr *port_mngr;
> + int ret;
> +
> + port_mngr = port->port_mngr;
> +
> + if (test_bit(PORT_S_ENABLE, &port->status))
> + return;
> +
> + ret = mtk_port_ch_enable(port);
> + if (ret && ret != -EBUSY)
> + return;
> +
> + port->w_priv.w_port = wwan_create_port(port_mngr->ctrl_blk->mdev->dev,
> + port->w_priv.w_type,
> + &wwan_ops, NULL, port);
> + if (IS_ERR(port->w_priv.w_port)) {
[Severity: High]
Can a race condition occur here if wwan_create_port() fails and returns an
error pointer?
Once mtk_port_ch_enable() completes, hardware RX can start. If
wwan_create_port() fails, an ERR_PTR is stored in port->w_priv.w_port
without holding w_lock. Concurrently, the RX interrupt handler could call
mtk_port_wwan_recv():
mutex_lock(&port->w_priv.w_lock);
if (!port->w_priv.w_port) {
mutex_unlock(&port->w_priv.w_lock);
return -ENXIO;
}
wwan_port_rx(port->w_priv.w_port, skb);
Because an ERR_PTR is not NULL, mtk_port_wwan_recv() will pass it to
wwan_port_rx(), which will then dereference the error pointer.
> + dev_warn(port_mngr->ctrl_blk->mdev->dev,
> + "Failed to create wwan port for (%s)\n", port->info.name);
> + port->w_priv.w_port = NULL;
> + mtk_port_ch_disable(port);
> + return;
> + }
> +
> + set_bit(PORT_S_WR, &port->status);
> + set_bit(PORT_S_ENABLE, &port->status);
> +}
> +
^ permalink raw reply
* Re: [PATCH v4 5/7] net: wwan: t9xx: Add FSM thread
From: Simon Horman @ 2026-07-16 9:34 UTC (permalink / raw)
To: jackbb_wu
Cc: Simon Horman, loic.poulain, ryazanov.s.a, johannes, andrew+netdev,
davem, edumazet, kuba, pabeni, wen-zhi.huang, shi-wei.yeh,
Minano.tseng, matthias.bgg, angelogioacchino.delregno, corbet,
skhan, linux-kernel, netdev, linux-arm-kernel, linux-mediatek,
linux-doc
In-Reply-To: <20260709-t9xx_driver_v1-v4-5-a8c009d509c5@compal.com>
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
---
--- Patch [5]: [PATCH v4 5/7] net: wwan: t9xx: Add FSM thread ---
commit 8cf81b126e77414e72ef40886e0728c165b2e588
Author: Jack Wu <jackbb_wu@compal.com>
net: wwan: t9xx: Add FSM thread
This commit introduces a finite-state machine (FSM) thread for the MTK WWAN
driver to synchronize module actions. It handles events sequentially from a
queue and notifies other modules before and after state transitions.
> diff --git a/drivers/net/wwan/t9xx/mtk_fsm.c b/drivers/net/wwan/t9xx/mtk_fsm.c
> new file mode 100644
> index 0000000000000..daefb7581ce0f
> --- /dev/null
> +++ b/drivers/net/wwan/t9xx/mtk_fsm.c
[ ... ]
> +static int mtk_fsm_parse_hs2_msg(struct fsm_hs_info *hs_info)
> +{
> + struct mtk_md_fsm *fsm = container_of(hs_info, struct mtk_md_fsm, hs_info[hs_info->id]);
> + char *rt_data = ((struct sk_buff *)hs_info->rt_data)->data;
[Severity: High]
Does this iterate over the SKB payload assuming the entire payload is linearly
accessible? Networking core guidelines usually require checking boundaries
using pskb_may_pull() before direct dereferencing, as packets might be
fragmented.
[ ... ]
> + if (cur_ft_spt == RTFT_TYPE_MUST_SUPPORT)
> + if (query_rtft_action[ft_id])
> + ret = query_rtft_action[ft_id](fsm->mdev, rtft_entry->data);
> + if (ret < 0)
> + break;
> +
> + data_len = le32_to_cpu(rtft_entry->data_len);
> + if (data_len > hs_info->rt_data_len - offset - sizeof(*rtft_entry))
> + break;
[Severity: High]
Is the rtft_entry->data passed to the action callback (like
mtk_port_status_update) before data_len is checked against the remaining
buffer bounds? This appears to allow an out-of-bounds heap read if a
crafted message has a large port_cnt and truncated payload.
[ ... ]
> +static int mtk_fsm_send_hs3_msg(struct fsm_hs_info *hs_info)
> +{
> + struct mtk_md_fsm *fsm = container_of(hs_info, struct mtk_md_fsm, hs_info[hs_info->id]);
> + unsigned int data_len, msg_size = 0;
> + struct ctrl_msg_header *ctrl_msg_h;
> + struct sk_buff *skb;
> + int ret;
> +
> + skb = __dev_alloc_skb(RTFT_DATA_SIZE, GFP_KERNEL);
> + if (!skb)
> + return -ENOMEM;
[Severity: High]
Does this leak uninitialized kernel heap memory to the WWAN device?
__dev_alloc_skb() does not zero the buffer, and when the structure is
populated in mtk_fsm_append_rtft_entries(), the 2-byte reserved field
(and potentially support_info if the switch falls through) appear to be
left uninitialized.
[ ... ]
> +static int mtk_fsm_sap_ctrl_msg_handler(void *__fsm, struct sk_buff *skb)
> +{
> + struct ctrl_msg_header *ctrl_msg_h;
> + struct mtk_md_fsm *fsm = __fsm;
> + struct fsm_hs_info *hs_info;
> + int ret;
> +
> + if (skb->len < sizeof(*ctrl_msg_h)) {
> + dev_kfree_skb(skb);
> + return -EINVAL;
> + }
> +
> + ctrl_msg_h = (struct ctrl_msg_header *)skb->data;
> + skb_pull(skb, sizeof(*ctrl_msg_h));
[Severity: High]
Can skb_pull() trigger a BUG() here if the SKB is fragmented? It seems the
payload is directly accessed and pulled without calling pskb_may_pull()
to guarantee the required bytes are in the linear data region.
[ ... ]
> + hs_info = &fsm->hs_info[HS_ID_SAP];
> + if (le32_to_cpu(ctrl_msg_h->id) != CTRL_MSG_HS2) {
> + dev_kfree_skb(skb);
> + return -EPROTO;
> + }
> +
> + hs_info->rt_data = skb;
> + hs_info->rt_data_len = skb->len;
> + ret = mtk_fsm_evt_submit(fsm->mdev, FSM_EVT_STARTUP,
> + hs_info->fsm_flag_hs2, hs_info, sizeof(*hs_info), 0);
> + if (ret == FSM_EVT_RET_FAIL)
> + dev_kfree_skb(skb);
[Severity: High]
If mtk_fsm_evt_submit() fails, the SKB is freed, but hs_info->rt_data
is not set to NULL. Could this dangling pointer cause a double-free later in
the error path of mtk_fsm_startup_act()?
Also, hs_info->rt_data is assigned without locking here. If consecutive
messages arrive, could this overwrite the pointer and leak the previous SKB?
When the FSM thread eventually parses the first event, it sets the pointer to
NULL, which would cause subsequent queued events to dereference NULL in
mtk_fsm_parse_hs2_msg().
[ ... ]
> +static void mtk_fsm_evt_cleanup(struct mtk_md_fsm *fsm, struct list_head *evtq)
> +{
> + struct mtk_fsm_evt *event, *tmp;
> +
> + list_for_each_entry_safe(event, tmp, evtq, entry) {
> + list_del(&event->entry);
> + mtk_fsm_evt_finish(fsm, event, FSM_EVT_RET_FAIL);
> + }
> +}
[Severity: Medium]
When flushing the event queue on device removal, does this cleanup miss
freeing the SKB currently stored in the shared fsm->hs_info[].rt_data if an
HS2 message was pending?
[ ... ]
> +int mtk_fsm_notifier_register(struct mtk_md_dev *mdev, enum mtk_user_id id,
> + void (*cb)(struct mtk_fsm_param *, void *data),
> + void *data, enum mtk_fsm_prio prio, bool is_pre)
> +{
[ ... ]
> + if (is_pre)
> + mtk_fsm_notifier_insert(notifier, &fsm->pre_notifiers);
> + else
> + mtk_fsm_notifier_insert(notifier, &fsm->post_notifiers);
[Severity: Medium]
Is it safe to modify these notifier lists globally without spinlock or mutex
protection? The FSM kthread traverses these lists in mtk_fsm_switch_state()
and could race with registration or unregistration.
[ ... ]
> +int mtk_fsm_evt_submit(struct mtk_md_dev *mdev,
> + enum mtk_fsm_evt_id id, enum mtk_fsm_flag flag,
> + void *data, unsigned int len, unsigned char mode)
> +{
[ ... ]
> + spin_unlock_irqrestore(&fsm->evtq_lock, flags);
> +
> + wake_up_process(fsm->fsm_handler);
[Severity: High]
Could a race condition occur here during teardown? If this thread is
preempted right after releasing the lock, mtk_fsm_exit() could execute,
stopping the kthread and setting fsm->fsm_handler = NULL. When this thread
resumes, calling wake_up_process(NULL) would trigger a kernel panic.
> diff --git a/drivers/net/wwan/t9xx/mtk_port.c b/drivers/net/wwan/t9xx/mtk_port.c
> --- a/drivers/net/wwan/t9xx/mtk_port.c
> +++ b/drivers/net/wwan/t9xx/mtk_port.c
[ ... ]
> int mtk_port_ch_disable(struct mtk_port *port)
> {
[ ... ]
> start_wait:
> ret = wait_event_interruptible_timeout(port->trb_wq, trb->status <= 0,
> MTK_DFLT_TRB_TIMEOUT);
> if (ret == -ERESTARTSYS)
> goto start_wait;
[Severity: High]
If a signal is pending (e.g., SIGKILL), wait_event_interruptible_timeout()
immediately returns -ERESTARTSYS. Won't this retry loop cause an infinite
busy loop (100% CPU lockup) because the signal remains pending without
sleeping?
[ ... ]
> +static void mtk_port_disable(struct mtk_port_mngr *port_mngr)
> +{
> + struct mtk_port **ports;
> + int tbl_type;
> + int ret, idx;
> +
> + ports = kcalloc(port_mngr->port_cnt, sizeof(struct mtk_port *), GFP_KERNEL);
> + if (!ports)
> + return;
[Severity: High]
If kcalloc() fails under memory pressure, this aborts the disable sequence
silently. Would this leave hardware channels active during device teardown
and cause use-after-free crashes? It might be safer to iterate via
radix_tree_for_each_slot() without dynamic allocation.
^ permalink raw reply
* Re: [PATCH v4 4/7] net: wwan: t9xx: Add control port
From: Simon Horman @ 2026-07-16 9:34 UTC (permalink / raw)
To: jackbb_wu
Cc: Simon Horman, loic.poulain, ryazanov.s.a, johannes, andrew+netdev,
davem, edumazet, kuba, pabeni, wen-zhi.huang, shi-wei.yeh,
Minano.tseng, matthias.bgg, angelogioacchino.delregno, corbet,
skhan, linux-kernel, netdev, linux-arm-kernel, linux-mediatek,
linux-doc
In-Reply-To: <20260709-t9xx_driver_v1-v4-4-a8c009d509c5@compal.com>
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
---
--- Patch [4]: [PATCH v4 4/7] net: wwan: t9xx: Add control port ---
commit 8d0ed42502a8e2e3fe15beabcf8bda318350f746
Author: Jack Wu <jackbb_wu@compal.com>
Date: Thu Jul 9 10:53:26 2026 +0000
net: wwan: t9xx: Add control port
The control port consists of port I/O and port manager.
Port I/O provides a common operation as defined by "struct port_ops",
and the operation is managed by the "port manager". It provides
interfaces to internal users, the implemented internal interfaces are
open, close, write and recv_register.
> diff --git a/drivers/net/wwan/t9xx/mtk_port.c b/drivers/net/wwan/t9xx/mtk_port.c
> new file mode 100644
> index 0000000000000..06436422133ba
> --- /dev/null
> +++ b/drivers/net/wwan/t9xx/mtk_port.c
> @@ -0,0 +1,875 @@
[ ... ]
> +static void mtk_port_free_or_backup(struct mtk_port_mngr *port_mngr,
> + struct mtk_port *port, struct mtk_stale_list *s_list)
> +{
> + mutex_lock(&port_mngr_grp_mtx);
> + mtk_port_tbl_del(port_mngr, port);
> + if (port->info.type != PORT_TYPE_INTERNAL) {
> + if (test_bit(PORT_S_OPEN, &port->status)) {
> + list_add_tail(&port->stale_entry, &s_list->ports);
> + set_bit(PORT_S_ON_STALE_LIST, &port->status);
> + memcpy(port->dev_str, port_mngr->ctrl_blk->mdev->dev_str,
> + MTK_DEV_STR_LEN);
> + port->port_mngr = NULL;
> + }
> + kref_put(&port->kref, mtk_port_release);
> + } else {
> + mtk_port_release(&port->kref);
[Severity: High]
Does this directly bypass the kref API? By explicitly calling
mtk_port_release() rather than kref_put(), this forces the port to be freed
immediately, ignoring any active references internal clients might still hold.
Could this lead to a use-after-free during device removal or module unload?
> + }
> + mutex_unlock(&port_mngr_grp_mtx);
> +}
> +
> +static struct mtk_port *mtk_port_search_by_id(struct mtk_port_mngr *port_mngr, int rx_ch)
> +{
> + int tbl_type = MTK_PORT_TBL_TYPE(rx_ch);
> +
> + if (tbl_type < PORT_TBL_SAP || tbl_type >= PORT_TBL_MAX)
> + return NULL;
> +
> + return radix_tree_lookup(&port_mngr->port_tbl[tbl_type], MTK_CH_ID(rx_ch));
[Severity: High]
Is it safe to perform radix tree lookups without an RCU read-side critical
section?
Since mtk_port_release() uses a synchronous kfree() instead of kfree_rcu(),
if a port closure or device removal occurs concurrently, the traversal here
might access freed radix tree nodes or port structures.
> +}
> +
> +struct mtk_port *mtk_port_search_by_name(struct mtk_port_mngr *port_mngr, char *name)
> +{
> + int tbl_type = PORT_TBL_SAP;
> + struct radix_tree_iter iter;
> + struct mtk_port *port;
> + void __rcu **slot;
> +
> + do {
> + radix_tree_for_each_slot(slot, &port_mngr->port_tbl[tbl_type], &iter, 0) {
[Severity: High]
Similarly, does this radix tree iteration require rcu_read_lock() protection
to prevent traversing into memory that has been concurrently freed?
> + MTK_PORT_SEARCH_FROM_RADIX_TREE(port, slot);
> + MTK_PORT_INTERNAL_NODE_CHECK(port, slot, iter);
> + if (!strncmp(port->info.name, name, MTK_DFLT_PORT_NAME_LEN))
> + return port;
> + }
> + tbl_type++;
> + } while (tbl_type < PORT_TBL_MAX);
> +
> + return NULL;
> +}
[ ... ]
> +void mtk_port_stale_list_grp_cleanup(void)
> +{
> + struct mtk_stale_list *s_list, *next_s_list;
> + struct mtk_port *port, *next_port;
> +
> + mutex_lock(&port_mngr_grp_mtx);
> + list_for_each_entry_safe(s_list, next_s_list, &stale_list_grp, entry) {
> + list_del(&s_list->entry);
> +
> + list_for_each_entry_safe(port, next_port, &s_list->ports, stale_entry) {
> + clear_bit(PORT_S_ON_STALE_LIST, &port->status);
> + mtk_port_release(&port->kref);
[Severity: High]
This appears to be another instance of bypassing the kref API by calling
mtk_port_release() directly instead of kref_put().
> + }
> +
> + kfree(s_list);
[Severity: Medium]
Does this leak the s_list->dev_id? The list entries are freed here during
module unload, but there doesn't appear to be a corresponding call to
ida_free() to return the dev_id to the allocator.
> + }
> + mutex_unlock(&port_mngr_grp_mtx);
> +}
[ ... ]
> +void mtk_port_trb_init(struct mtk_port *port, struct trb *trb, enum mtk_trb_cmd_type cmd,
> + int (*trb_complete)(struct sk_buff *skb))
> +{
> + kref_init(&trb->kref);
> + trb->channel_id = port->info.rx_ch;
> + trb->status = MTK_DFLT_TRB_STATUS;
> + trb->priv = port;
[Severity: High]
Does this assignment need to take a reference to the port with kref_get()?
If a non-blocking transmission is submitted, the port could be closed and
freed while the TRB is still in the hardware DMA ring. When the DMA
interrupt finally fires, the async completion handler mtk_port_tx_complete()
will access trb->priv, potentially dereferencing a freed port pointer.
> + trb->cmd = cmd;
> + trb->trb_complete = trb_complete;
> +}
[ ... ]
> +int mtk_port_send_data(struct mtk_port *port, void *data)
> +{
> + struct mtk_port_mngr *port_mngr;
> + struct sk_buff *skb = data;
> + bool force_send;
> + struct trb *trb;
> + int ret, len;
> +
> + port_mngr = port->port_mngr;
> +
> + force_send = !!(port->info.flags & (PORT_F_BLOCKING | PORT_F_FORCE_SEND));
> + trb = (struct trb *)skb->cb;
> + mtk_port_trb_init(port, trb, TRB_CMD_TX, mtk_port_tx_complete);
> + len = skb->len;
> + kref_get(&trb->kref); /* kref count 1->2 */
> +
> + /* add ccci header */
> + mtk_port_add_header(skb);
> + ret = mtk_port_status_check(port);
> + if (!ret)
> + ret = port_mngr->ctrl_blk->ops->submit_skb(port_mngr->ctrl_blk->mdev,
> + skb, force_send);
> +
> + if (ret < 0) {
> + kref_put(&trb->kref, mtk_port_trb_free); /* kref count 2->1 */
> + kref_put(&trb->kref, mtk_port_trb_free); /* kref count 1->0 */
> + port->tx_seq--;
[Severity: Medium]
Can concurrent writers corrupt the tx_seq variable?
Because the mtk_port_internal_write() API doesn't seem to enforce locking,
multiple threads transmitting on the same port can induce a read-modify-write
race on this variable here and in mtk_port_add_header(). This could result in
duplicate or out-of-order sequence numbers being sent to the WWAN modem.
> + goto out;
> + }
[ ... ]
> +static int mtk_port_rx_dispatch_frag_skb(struct mtk_port *port, struct sk_buff *skb)
> +{
> + struct sk_buff *frag_skb, *frag_next;
> + int ret;
> +
> + frag_skb = skb_shinfo(skb)->frag_list;
> + skb->len -= skb->data_len;
> + skb->data_len = 0;
> + skb_shinfo(skb)->frag_list = NULL;
> +
> + ret = ports_ops[port->info.type]->recv(port, skb);
> + if (ret < 0) {
> + skb_shinfo(skb)->frag_list = frag_skb;
[Severity: High]
Can this lead to a use-after-free and a double-free on the skb?
If the port is not in an open state, mtk_port_internal_recv() explicitly calls
dev_kfree_skb_any(skb) and returns -ENXIO:
mtk_port_internal_recv() {
if (!test_bit(PORT_S_OPEN, &port->status))
goto drop_data;
...
drop_data:
dev_kfree_skb_any(skb);
return ret;
}
When the error returns here to mtk_port_rx_dispatch_frag_skb(), it accesses
the freed skb to update frag_list. It then propagates the error up to
mtk_port_rx_dispatch(), which jumps to the drop_data label and frees the skb
a second time.
> + return ret;
> + }
[ ... ]
> +static int mtk_port_rx_dispatch(struct sk_buff *skb, void *priv, bool force_recv)
> +{
[ ... ]
> + /* Support scatter gather transmission */
> + if (port->rx_mtu > port->rx_frag_size) {
> + ret = mtk_port_rx_dispatch_frag_skb(port, skb);
> + /* -EIO means partial data dispatch complete, does not goto drop flow */
> + if (ret < 0 && ret != -EIO)
> + goto drop_frag_skb;
> + } else {
> + ret = ports_ops[port->info.type]->recv(port, skb);
> + if (ret < 0)
> + goto drop_data;
> + }
> +
> + return ret;
> +
> +drop_frag_skb:
> + {
> + struct sk_buff *frag_skb, *tmp;
> +
> + frag_skb = skb_shinfo(skb)->frag_list;
> + while (frag_skb) {
> + tmp = frag_skb->next;
> + frag_skb->next = NULL;
> + dev_kfree_skb_any(frag_skb);
> + frag_skb = tmp;
> + }
> + skb_shinfo(skb)->frag_list = NULL;
> + }
> +drop_data:
> + dev_kfree_skb_any(skb);
[Severity: High]
This is where the second free of the skb occurs when propagating the error
from the recv callback.
> + return ret;
> +}
> +
> +int mtk_port_add_header(struct sk_buff *skb)
> +{
[ ... ]
> + ccci_h->packet_header = cpu_to_le32(0);
> + ccci_h->packet_len = cpu_to_le32(skb->len);
> + ccci_h->ex_msg = cpu_to_le32(0);
> + ccci_h->status = cpu_to_le32(FIELD_PREP(MTK_HDR_FLD_CHN, port->info.tx_ch) |
> + FIELD_PREP(MTK_HDR_FLD_SEQ, port->tx_seq++) |
[Severity: Medium]
This is the other side of the tx_seq data race where concurrent writers will
corrupt the sequence tracking.
> + FIELD_PREP(MTK_HDR_FLD_AST, 1));
> +
> + trb->status = MTK_TRB_HEADER_ADDED;
> +
> + return 0;
> +}
[ ... ]
> +int mtk_port_status_update(struct mtk_md_dev *mdev, void *data)
> +{
> + struct mtk_port_enum_msg *msg = data;
> + struct mtk_port_info *port_info;
> + struct mtk_port_mngr *port_mngr;
> + struct mtk_ctrl_blk *ctrl_blk;
> + struct mtk_port *port;
> + int port_id;
> + u16 ch_id;
> +
> + if (unlikely(!mdev || !msg))
> + return -EINVAL;
> +
> + ctrl_blk = mdev->ctrl_blk;
> + port_mngr = ctrl_blk->port_mngr;
> + if (le16_to_cpu(msg->version) != MTK_PORT_ENUM_VER ||
> + le32_to_cpu(msg->head_pattern) != MTK_PORT_ENUM_HEAD_PATTERN ||
> + le32_to_cpu(msg->tail_pattern) != MTK_PORT_ENUM_TAIL_PATTERN)
> + return -EPROTO;
> +
> + for (port_id = 0; port_id < le16_to_cpu(msg->port_cnt); port_id++) {
[Severity: High]
Is there a missing bounds check on msg->port_cnt here?
This loop relies strictly on the device-provided port_cnt value without
validating it against the actual size of the received msg->data payload.
A malformed or maliciously large count from the WWAN firmware could cause an
out-of-bounds memory read.
> + port_info = (struct mtk_port_info *)(msg->data +
> + (sizeof(*port_info) * port_id));
> + ch_id = FIELD_GET(MTK_INFO_FLD_CHID, le16_to_cpu(port_info->channel));
> + port = mtk_port_search_by_id(port_mngr, ch_id);
> + if (!port)
> + continue;
> + port->enable = FIELD_GET(MTK_INFO_FLD_EN, le16_to_cpu(port_info->channel));
> + }
> +
> + return 0;
> +}
> +
> +int mtk_port_ch_enable(struct mtk_port *port)
> +{
[ ... ]
> +start_wait:
> + ret = wait_event_interruptible_timeout(port->trb_wq, trb->status <= 0,
> + MTK_DFLT_TRB_TIMEOUT);
> + if (ret == -ERESTARTSYS)
> + goto start_wait;
[Severity: High]
Could this create an infinite loop in kernel space?
If a process executing this function has a pending signal (such as being
killed), wait_event_interruptible_timeout() returns -ERESTARTSYS immediately
without sleeping. The unconditional goto forces a tight loop that never
returns to userspace to process the signal.
> + else if (!ret)
> + ret = -ETIMEDOUT;
> + else
> + ret = trb->status;
> +
> + kref_put(&trb->kref, mtk_port_trb_free);
> +
> + return ret;
> +}
> +
> +int mtk_port_ch_disable(struct mtk_port *port)
> +{
[ ... ]
> +start_wait:
> + ret = wait_event_interruptible_timeout(port->trb_wq, trb->status <= 0,
> + MTK_DFLT_TRB_TIMEOUT);
> + if (ret == -ERESTARTSYS)
> + goto start_wait;
[Severity: High]
This appears to have the same infinite loop risk as mtk_port_ch_enable() when
a signal is pending.
> + else if (!ret)
> + ret = -ETIMEDOUT;
> + else
> + ret = trb->status;
> +
> + kref_put(&trb->kref, mtk_port_trb_free);
> +
> + return ret;
> +}
> diff --git a/drivers/net/wwan/t9xx/mtk_port_io.c b/drivers/net/wwan/t9xx/mtk_port_io.c
> new file mode 100644
> index 0000000000000..e3a2de6d2f297
> --- /dev/null
> +++ b/drivers/net/wwan/t9xx/mtk_port_io.c
> @@ -0,0 +1,239 @@
[ ... ]
> +static int mtk_port_internal_recv(struct mtk_port *port, struct sk_buff *skb)
> +{
> + struct mtk_internal_port *priv;
> + int ret = -ENXIO;
> +
> + if (!test_bit(PORT_S_OPEN, &port->status))
> + goto drop_data;
> +
> + priv = &port->i_priv;
> + if (!priv->recv_cb || !priv->arg)
> + goto drop_data;
> +
> + ret = priv->recv_cb(priv->arg, skb);
> + return ret;
> +
> +drop_data:
> + dev_kfree_skb_any(skb);
[Severity: High]
This is where the skb is freed on the error path before returning -ENXIO,
leading to the use-after-free and double-free in the caller.
> + return ret;
> +}
> +
> +static int mtk_port_common_open(struct mtk_port *port)
> +{
> + int ret = 0;
> +
> + if (!test_bit(PORT_S_ENABLE, &port->status))
> + return -ENODEV;
> +
> + if (test_bit(PORT_S_OPEN, &port->status))
> + return -EBUSY;
> +
> + skb_queue_purge(&port->rx_skb_list);
> + set_bit(PORT_S_OPEN, &port->status);
[Severity: High]
Is this test_bit() and set_bit() sequence intended to provide mutual
exclusion?
Since this is not an atomic test-and-set operation, two threads could
concurrently observe PORT_S_OPEN as unset, simultaneously execute
skb_queue_purge() which corrupts the queue, and both proceed believing
they have successfully opened the port.
> + clear_bit(PORT_S_FLUSH, &port->status);
> +
> + return ret;
> +}
[ ... ]
^ permalink raw reply
* Re: [PATCH v4 3/7] net: wwan: t9xx: Add control DMA interface
From: Simon Horman @ 2026-07-16 9:34 UTC (permalink / raw)
To: jackbb_wu
Cc: Simon Horman, loic.poulain, ryazanov.s.a, johannes, andrew+netdev,
davem, edumazet, kuba, pabeni, wen-zhi.huang, shi-wei.yeh,
Minano.tseng, matthias.bgg, angelogioacchino.delregno, corbet,
skhan, linux-kernel, netdev, linux-arm-kernel, linux-mediatek,
linux-doc
In-Reply-To: <20260709-t9xx_driver_v1-v4-3-a8c009d509c5@compal.com>
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
---
--- Patch [3]: [PATCH v4 3/7] net: wwan: t9xx: Add control DMA interface ---
commit ee2f68157a30f68c2d68e46c572820b187896695
Author: Jack Wu <jackbb_wu@compal.com>
net: wwan: t9xx: Add control DMA interface
This patch adds the Cross Layer Direct Memory Access (CLDMA) interface for
the control plane to translate data between the host and device. It
implements GPD ring traversal for TX and RX paths and hardware interrupt
handling.
> diff --git a/drivers/net/wwan/t9xx/pcie/mtk_cldma.c b/drivers/net/wwan/t9xx/pcie/mtk_cldma.c
> index 0000000000000..3bab14d35d78d
> --- /dev/null
> +++ b/drivers/net/wwan/t9xx/pcie/mtk_cldma.c
> [ ... ]
> +static void mtk_cldma_tx_done_work(struct work_struct *work)
> +{
> [ ... ]
> + req = txq->req_pool + txq->free_idx;
> +
> + rmb(); /* ensure HWO setup done before HWO read */
> +
> + if (!req->data_vm_addr || (req->gpd->tx_gpd.gpd_flags & CLDMA_GPD_FLAG_HWO))
> + break;
[Severity: High]
Is a dma_rmb() needed after checking the hardware ownership flag? On weakly
ordered architectures, speculative execution might read payload or length
fields before the ownership check is fully ordered in memory.
> [ ... ]
> +static void mtk_cldma_rx_done_work(struct work_struct *work)
> +{
> [ ... ]
> +again:
> + for (i = 0; i < rxq->nr_gpds; i++) {
> + req = rxq->req_pool + rxq->free_idx;
> + if (!req->skb) {
> [ ... ]
> + }
> +
> + if (req->gpd->rx_gpd.gpd_flags & CLDMA_GPD_FLAG_HWO)
> + break;
> +
> + mtk_cldma_rx_skb_adjust(mdev, rxq, req);
[Severity: High]
Similarly, would we need a dma_rmb() here before attempting to read the
receive length out of the descriptor in mtk_cldma_rx_skb_adjust?
> + do {
> + ret = rxq->rx_done(req->skb, rxq->arg,
> + atomic_read(&rxq->need_exit) ? true : false);
> + if (ret == -EAGAIN)
> + usleep_range(1000, 2000);
> + else
> + req->skb = NULL;
> + } while (ret == -EAGAIN);
> +
> + ret = mtk_cldma_reload_rx_skb(mdev, rxq, req);
> + if (ret)
> + goto out;
[Severity: High]
If mtk_cldma_reload_rx_skb fails due to memory pressure, it jumps to out
without advancing rxq->free_idx. Will this cause the worker to permanently
stall the queue, as subsequent runs will constantly evaluate the same slot
without being able to make progress?
> [ ... ]
> +static void mtk_cldma_txq_free(struct cldma_drv_info *drv_info, u32 txqno)
> +{
> [ ... ]
> + for (j = 0; j < txq->nr_bds; j++) {
> + bd_dsc = req->bd_dsc_pool + j;
> + dma_pool_free(drv_info->bd_dma_pool, bd_dsc->bd,
> + bd_dsc->bd_dma_addr);
> + }
> + kfree(req->bd_dsc_pool);
> + dma_pool_free(drv_info->gpd_dma_pool, req->gpd, req->gpd_dma_addr);
> + txq->free_idx = (txq->free_idx + 1) % txq->nr_gpds;
> + }
> +
> + kfree(txq->req_pool);
> + kfree(txq);
> +}
[Severity: Critical]
Does this incorrectly free memory managed by devres? The arrays req_pool and
bd_dsc_pool are allocated using devm_kcalloc during initialization.
Calling kfree directly on devres-managed memory causes slab allocator
corruption and later double-frees when the device is detached. This same
pattern appears to be present in mtk_cldma_rxq_free as well.
> [ ... ]
> +static int mtk_cldma_reload_rx_skb(struct mtk_md_dev *mdev, struct rxq *rxq,
> + struct rx_req *req)
> +{
> [ ... ]
> +err_free_skb:
> + if (nr_bds) {
> [ ... ]
> + for (i = 0; i < nr_bds; i++) {
> + bd_dsc = req->bd_dsc_pool + i;
> + if (!bd_dsc->skb)
> + break;
> + if (!dma_mapping_error(mdev->dev, bd_dsc->data_dma_addr))
> + dma_unmap_single(mdev->dev, bd_dsc->data_dma_addr,
> + req->frag_size, DMA_FROM_DEVICE);
> + bd_dsc->data_dma_addr = 0;
> + bd_dsc->skb->next = NULL;
> + dev_kfree_skb_any(bd_dsc->skb);
[Severity: High]
Can this lead to a double free? After calling dev_kfree_skb_any here, the
pointer bd_dsc->skb is not set to NULL. During driver teardown,
mtk_cldma_rxq_free will iterate over this array, observe bd_dsc->skb is
still populated, and attempt to free it a second time.
> [ ... ]
> +static int mtk_cldma_txbuf_set(struct cldma_drv_info *drv_info, struct sk_buff *skb,
> + struct tx_req *req, int nr_bds)
> +{
> [ ... ]
> + } else {
> + req->data_dma_addr = dma_map_single(mdev->dev, skb->data,
> + skb->len, DMA_TO_DEVICE);
[Severity: High]
Does this code safely handle non-linear SKBs? When nr_bds is 0, this maps
skb->data using the full skb->len instead of skb_headlen(skb). Could this
cause an out-of-bounds read past the linear buffer for fragmented packets?
> [ ... ]
> +int mtk_cldma_submit_tx(void *dev, struct sk_buff *skb)
> +{
> [ ... ]
> + req->data_len = skb->len;
> + req->skb = skb;
> + req->data_vm_addr = skb->data;
> +
> + wmb(); /* ensure req and data msg set done before HWO setup */
> +
> + req->gpd->tx_gpd.gpd_flags |= CLDMA_GPD_FLAG_HWO;
[Severity: High]
Could there be a race condition here with mtk_cldma_tx_done_work?
By setting req->data_vm_addr before setting the CLDMA_GPD_FLAG_HWO flag, a
concurrent execution of the completion worker might observe data_vm_addr as
non-NULL but with the hardware ownership flag still cleared, misinterpreting
this intermediate state as a completed transmission.
> diff --git a/drivers/net/wwan/t9xx/pcie/mtk_pci.c b/drivers/net/wwan/t9xx/pcie/mtk_pci.c
> index 68dddd652effc..2f51e150750da 100644
> --- a/drivers/net/wwan/t9xx/pcie/mtk_pci.c
> +++ b/drivers/net/wwan/t9xx/pcie/mtk_pci.c
> [ ... ]
> @@ -998,6 +1036,7 @@ static void mtk_pci_remove(struct pci_dev *pdev)
> }
>
> pci_clear_master(pdev);
> + mtk_pci_dev_exit(mdev);
> mtk_pci_free_irq(mdev);
[Severity: High]
Is it intended to call pci_clear_master before mtk_pci_dev_exit? Disabling
bus mastering before the hardware queues are fully shut down via MMIO
commands in mtk_pci_dev_exit might block pending hardware DMA transactions
or completion descriptors, which can lead to PCIe Master Aborts or hangs.
> diff --git a/drivers/net/wwan/t9xx/pcie/mtk_trans_ctrl.c b/drivers/net/wwan/t9xx/pcie/mtk_trans_ctrl.c
> index 0000000000000..55297cfcdbc61
> --- /dev/null
> +++ b/drivers/net/wwan/t9xx/pcie/mtk_trans_ctrl.c
> [ ... ]
> +static void mtk_ctrl_trb_handler(struct trb_srv *srv, struct trans_list *trans_list, u32 qno)
> +{
> [ ... ]
> + case TRB_CMD_TX:
> + err = mtk_cldma_submit_tx(trans->dev, skb);
> + if (err) {
> + if (trans_list->tx_burst_cnt[qno]) {
> + kick = true;
> + break;
> + }
> + if (err == -EAGAIN)
> + return;
[Severity: High]
If mtk_cldma_submit_tx returns -EAGAIN due to DMA mapping failures, this
returns immediately and leaves the skb on the list. Since the budget is not
consumed and the queue remains non-empty, mtk_ctrl_chs_is_busy_or_empty
will evaluate to false. Could this cause the mtk_ctrl_trb_thread loop to
spin continuously without sleeping, leading to a CPU lockup?
> [ ... ]
> +static int mtk_ctrl_trb_thread(void *args)
> +{
> + struct trb_srv *srv = args;
> +
> + for (;;) {
> + wait_event_interruptible(srv->trb_waitq,
> + !mtk_ctrl_chs_is_busy_or_empty(srv) ||
> + kthread_should_stop() || kthread_should_park());
[Severity: High]
Can this cause an infinite loop if a signal is delivered to the thread?
Using wait_event_interruptible without checking signal_pending(current) or
flushing signals means it will immediately return -ERESTARTSYS on every loop
iteration once a signal is pending, permanently burning a CPU core.
> [ ... ]
> +static void mtk_ctrl_remove_radix_tree(struct mtk_ctrl_trans *trans)
> +{
> + struct radix_tree_iter iter;
> + struct queue_info *queue;
> + void __rcu **slot;
> +
> + radix_tree_for_each_slot(slot, &trans->queue_tbl, &iter, 0) {
> + queue = radix_tree_deref_slot(slot);
> + if (!queue)
> + continue;
> + radix_tree_delete(&trans->queue_tbl, iter.index);
> + kfree(queue);
> + }
> +}
[Severity: High]
Is it safe to iterate over the radix tree and call radix_tree_delete without
holding rcu_read_lock? Because the tree operations use RCU for internal
node memory management, doing this locklessly might allow the grace period
to expire, causing a use-after-free when the iterator accesses nodes.
^ permalink raw reply
* Re: [PATCH v4 2/7] net: wwan: t9xx: Add control plane transaction layer
From: Simon Horman @ 2026-07-16 9:33 UTC (permalink / raw)
To: jackbb_wu
Cc: Simon Horman, loic.poulain, ryazanov.s.a, johannes, andrew+netdev,
davem, edumazet, kuba, pabeni, wen-zhi.huang, shi-wei.yeh,
Minano.tseng, matthias.bgg, angelogioacchino.delregno, corbet,
skhan, linux-kernel, netdev, linux-arm-kernel, linux-mediatek,
linux-doc
In-Reply-To: <20260709-t9xx_driver_v1-v4-2-a8c009d509c5@compal.com>
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
---
--- Patch [2]: [PATCH v4 2/7] net: wwan: t9xx: Add control plane transaction layer ---
commit a06b668b7ebf78322cb2957d8a6a11fdfb9235ad
Author: Jack Wu <jackbb_wu@compal.com>
net: wwan: t9xx: Add control plane transaction layer
The control plane implements TX services that reside in the
transaction layer. The services receive the packets from the
port layer and call the corresponding DMA components to
transmit data to the device. Meanwhile, TX services receive
and manage the port control commands from the port layer.
The control plane implements RX services that reside in the
transaction layer. The services receive the downlink packets
from the modem and transfer the packets to the corresponding
port layer interfaces.
> diff --git a/drivers/net/wwan/t9xx/mtk_ctrl_plane.c b/drivers/net/wwan/t9xx/mtk_ctrl_plane.c
> new file mode 100644
> index 0000000000000..cf6079218e1ce
> --- /dev/null
> +++ b/drivers/net/wwan/t9xx/mtk_ctrl_plane.c
> @@ -0,0 +1,45 @@
[ ... ]
> +int mtk_ctrl_init(struct mtk_md_dev *mdev)
> +{
> + struct mtk_ctrl_blk *ctrl_blk;
> +
> + ctrl_blk = devm_kzalloc(mdev->dev, sizeof(*ctrl_blk), GFP_KERNEL);
> + if (!ctrl_blk)
> + return -ENOMEM;
> +
> + ctrl_blk->mdev = mdev;
> + mdev->ctrl_blk = ctrl_blk;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(mtk_ctrl_init);
[Severity: Medium]
Does this patch actually implement the TX and RX services described in the
commit message?
The commit message claims to implement control plane TX and RX services, but
this code merely introduces empty initialization stubs (mtk_ctrl_init() and
mtk_ctrl_exit()) that are currently dead code.
> diff --git a/drivers/net/wwan/t9xx/pcie/mtk_pci.c b/drivers/net/wwan/t9xx/pcie/mtk_pci.c
> index 72259bf1b6033..68dddd652effc 100644
> --- a/drivers/net/wwan/t9xx/pcie/mtk_pci.c
> +++ b/drivers/net/wwan/t9xx/pcie/mtk_pci.c
> @@ -467,6 +468,7 @@ static u32 mtk_pci_ext_h2d_evt_hw_bits(u32 chs)
>
> SET_HW_BITS(hw_bits, chs, MHCCIF_RC2EP_EVT_DEVICE_RESET,
> DEV_EVT_H2D_DEVICE_RESET);
> +
> return LE32_TO_U32(cpu_to_le32(hw_bits));
> }
[Severity: High]
Does this code apply a double byte-swap on big-endian architectures?
Using cpu_to_le32() converts the bitmask, which is then cast to u32 and
passed to mtk_pci_write32() (wrapping iowrite32()). Because iowrite32()
intrinsically expects a host-endian value and applies its own conversion for
little-endian PCI memory, the value is byte-swapped twice, resulting in the
original big-endian layout being incorrectly written.
Additionally, this can cause ffs(hw_bits) in mtk_pci_send_ext_evt() to
calculate the wrong bit index on the already byte-swapped value.
^ permalink raw reply
* Re: [PATCH v4 1/7] net: wwan: t9xx: Add PCIe core
From: Simon Horman @ 2026-07-16 9:33 UTC (permalink / raw)
To: jackbb_wu
Cc: Simon Horman, loic.poulain, ryazanov.s.a, johannes, andrew+netdev,
davem, edumazet, kuba, pabeni, wen-zhi.huang, shi-wei.yeh,
Minano.tseng, matthias.bgg, angelogioacchino.delregno, corbet,
skhan, linux-kernel, netdev, linux-arm-kernel, linux-mediatek,
linux-doc
In-Reply-To: <20260709-t9xx_driver_v1-v4-1-a8c009d509c5@compal.com>
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
---
--- Patch [1]: [PATCH v4 1/7] net: wwan: t9xx: Add PCIe core ---
commit 4564b6641dc4f4a3fb3d169ca45443e5d18c66b7
Author: Jack Wu <jackbb_wu@compal.com>
net: wwan: t9xx: Add PCIe core
Registers the T900 device driver with the kernel. Set up all
the fundamental configurations for the device: PCIe layer,
Modem Host Cross Core Interface (MHCCIF), Reset Generation
Unit (RGU), modem common control operations and build
infrastructure.
> diff --git a/drivers/net/wwan/t9xx/pcie/mtk_pci.c b/drivers/net/wwan/t9xx/pcie/mtk_pci.c
> index 0000000000000..72259bf1b6033
> --- /dev/null
> +++ b/drivers/net/wwan/t9xx/pcie/mtk_pci.c
[ ... ]
> +static irqreturn_t mtk_pci_irq_msix(int irq, void *data)
> +{
> + struct mtk_pci_irq_desc *irq_desc = data;
> + struct mtk_md_dev *mdev = irq_desc->mdev;
> + struct mtk_pci_priv *priv;
> + u32 irq_state, irq_enable;
> +
> + priv = mdev->hw_priv;
> + irq_state = mtk_pci_mac_read32(priv, REG_MSIX_ISTATUS_HOST_GRP0_0);
> + irq_enable = mtk_pci_mac_read32(priv, REG_IMASK_HOST_MSIX_GRP0_0);
> + irq_state &= irq_enable;
> +
> + if (unlikely(!irq_state) ||
> + unlikely(!((irq_state & GENMASK(priv->irq_cnt - 1, 0)) &
> + irq_desc->msix_bits)))
> + return IRQ_NONE;
[Severity: High]
Does this logic silently drop hardware interrupts when fewer than 32 vectors are
allocated?
Because hardware source bits map to fixed high bit positions (such as bit 28
for MTK_IRQ_SRC_MHCCIF), systems that allocate fewer vectors will mask out
these high bits with GENMASK(priv->irq_cnt - 1, 0).
Evaluating !(0 & msix_bits) as true returns IRQ_NONE, which may eventually
disable the interrupt line.
[ ... ]
> +static int mtk_pci_request_irq_msix(struct mtk_md_dev *mdev,
> + int irq_cnt_allocated)
> +{
> + struct mtk_pci_priv *priv = mdev->hw_priv;
> + struct mtk_pci_irq_desc *irq_desc;
> + struct pci_dev *pdev;
> + int irq_cnt;
> + int ret, i;
> +
> + /* calculate the nearest 2's power number */
> + irq_cnt = BIT(fls(irq_cnt_allocated) - 1);
> + pdev = to_pci_dev(mdev->dev);
> + irq_desc = priv->irq_desc;
> + for (i = 0; i < irq_cnt; i++) {
> + irq_desc[i].mdev = mdev;
> + irq_desc[i].msix_bits = BIT(i);
> + snprintf(irq_desc[i].name, MTK_IRQ_NAME_LEN, "msix%d-%s", i, mdev->dev_str);
> + ret = pci_request_irq(pdev, i, mtk_pci_irq_msix, NULL,
> + &irq_desc[i], irq_desc[i].name);
[Severity: Medium]
Could this trigger a -Wformat-security compiler warning?
Passing a dynamic string variable directly as the format string to
pci_request_irq() can lead to format string vulnerabilities. Should this use
"%s", irq_desc[i].name instead?
[ ... ]
> +static u32 mtk_pci_ext_h2d_evt_hw_bits(u32 chs)
> +{
> + u32 hw_bits = 0;
> +
> + SET_HW_BITS(hw_bits, chs, MHCCIF_RC2EP_EVT_DEVICE_RESET,
> + DEV_EVT_H2D_DEVICE_RESET);
> + return LE32_TO_U32(cpu_to_le32(hw_bits));
> +}
[ ... ]
> +int mtk_pci_send_ext_evt(struct mtk_md_dev *mdev, u32 ch)
> +{
> + struct mtk_pci_priv *priv = mdev->hw_priv;
> + u32 rc_base, hw_bits;
> +
> + rc_base = priv->cfg->mhccif_rc_base_addr;
> +
> + /* Only allow one ch to be triggered at a time */
> + if (!is_power_of_2(ch)) {
> + dev_err(mdev->dev, "Unsupported ext evt ch=0x%08x\n", ch);
> + return -EINVAL;
> + }
> +
> + hw_bits = mtk_pci_ext_h2d_evt_hw_bits(ch);
> + mtk_pci_write32(mdev, rc_base + MHCCIF_RC2EP_SW_BSY, hw_bits);
> + mtk_pci_write32(mdev, rc_base + MHCCIF_RC2EP_SW_TCHNUM, ffs(hw_bits) - 1);
[Severity: Medium]
Will this break channel index calculation on big-endian systems?
If hw_bits has already been byte-swapped by cpu_to_le32() inside
mtk_pci_ext_h2d_evt_hw_bits(), performing ffs(hw_bits) - 1 on the byte-swapped
integer will result in a completely incorrect bit index.
[Severity: Medium]
What happens if the driver attempts to send an unmapped channel?
If ch is not mapped in mtk_pci_ext_h2d_evt_hw_bits(), hw_bits will evaluate
to 0. The calculation ffs(0) - 1 will then underflow to 0xFFFFFFFF. This
value is written directly to the hardware and might hang or crash the device
firmware.
[ ... ]
> +static int mtk_pci_bar_init(struct mtk_md_dev *mdev)
> +{
> + struct pci_dev *pdev = to_pci_dev(mdev->dev);
> + struct mtk_pci_priv *priv = mdev->hw_priv;
> + u32 bar[MTK_PCI_BAR_NUM];
> + int i, ret;
> +
> + for (i = 0; i < MTK_PCI_BAR_NUM; i++)
> + pci_read_config_dword(to_pci_dev(mdev->dev),
> + PCI_BASE_ADDRESS_0 + (i << 2), bar + i);
> +
> + ret = pcim_iomap_regions(pdev, MTK_REQUESTED_BARS, mdev->dev_str);
> + if (ret) {
> + dev_err(mdev->dev, "Failed to init MMIO. ret=%d\n", ret);
> + return ret;
> + }
> +
> + /* get ioremapped memory */
> + priv->mac_reg_base = pcim_iomap_table(pdev)[MTK_BAR_0_1_IDX];
> + priv->bar23_addr = pcim_iomap_table(pdev)[MTK_BAR_2_3_IDX];
> + if (!priv->mac_reg_base || !priv->bar23_addr) {
> + dev_err(mdev->dev, "Failed to init BAR.\n");
> + return -EINVAL;
> + }
> + /* We use MD view base address "0" to observe registers */
> + priv->ext_reg_base = priv->bar23_addr - ATR_PCIE_REG_TRSL_ADDR;
[Severity: Medium]
Is it safe to store an out-of-bounds pointer?
ATR_PCIE_REG_TRSL_ADDR is 0x10000000, and subtracting it here results in
priv->ext_reg_base pointing well outside the allocated object. This invokes
undefined behavior and might cause crashes or be optimized out by UBSAN.
Could the correct offset be calculated at the time of access instead?
[ ... ]
> +static void mtk_pci_remove(struct pci_dev *pdev)
> +{
> + struct mtk_md_dev *mdev = pci_get_drvdata(pdev);
> + struct mtk_pci_priv *priv = mdev->hw_priv;
> + struct device *dev = &pdev->dev;
> +
> + mtk_pci_mask_irq(mdev, priv->mhccif_irq_id);
> +
> + if (mtk_pci_pldr(mdev)) {
> + dev_warn(dev, "Failed to execute PLDR, try external event\n");
> + mtk_pci_reset(mdev, RESET_MHCCIF);
> + }
> +
> + pci_clear_master(pdev);
> + mtk_pci_free_irq(mdev);
> + mtk_mhccif_exit(mdev);
[Severity: High]
Does this sequence result in the device being powered off before the OS
interrupt handlers and workqueues are synchronized?
Calling mtk_pci_pldr() executes the ACPI PXP._OFF method, cutting power.
If mtk_mhccif_exit() (which calls cancel_work_sync(&priv->mhccif_work))
or mtk_pci_free_irq() runs after this, a concurrently executing workqueue
or interrupt handler might attempt to read MMIO registers from a powered-off
device, causing bus errors and kernel panics.
[ ... ]
> +static pci_ers_result_t mtk_pci_error_detected(struct pci_dev *pdev,
> + pci_channel_state_t state)
> +{
> + struct mtk_md_dev *mdev = pci_get_drvdata(pdev);
> +
> + dev_err(mdev->dev, "AER detected: pci_channel_state_t=%d\n", state);
> +
> + /* Request a slot reset. */
> + return PCI_ERS_RESULT_CAN_RECOVER;
> +}
[Severity: Medium]
Will returning PCI_ERS_RESULT_CAN_RECOVER actually abort the slot reset?
The comment says "Request a slot reset", but CAN_RECOVER instructs the PCI
core to skip the reset. Furthermore, since the driver provides no .resume or
.mmio_enabled callbacks, the device remains in a corrupted state despite
being marked as recovered.
Shouldn't this return PCI_ERS_RESULT_NEED_RESET?
^ permalink raw reply
* Re: net: rnpgbe: Pass an expression directly in rnpgbe_rm_adapter()
From: Dan Carpenter @ 2026-07-16 9:30 UTC (permalink / raw)
To: Markus Elfring
Cc: Uwe Kleine-König, netdev, kernel-janitors, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, MD Danish Anwar,
Michael Grzeschik, Paolo Abeni, Vadim Fedorenko, Yibo Dong, LKML,
Jonathan Corbet
In-Reply-To: <85ad473e-b2f5-4458-8973-a643f90eda5e@web.de>
On Wed, Jul 15, 2026 at 08:45:48PM +0200, Markus Elfring wrote:
>
> Jonathan Corbet provided the following information.
>
> “…
> But Herbert's patch added a line which dereferences the pointer prior to the check. That, of course, is a bug.
> …”
>
Notice that it says "dereferences" not "does pointer math".
>
> > Note, I didn't
> > study the C standard
>
> I hope that clarification approaches can evolve further according to this information source.
>
>
> > if the compiler is free to optimize out
> > do_something() also in the 2nd case, but at least today gcc doesn't.
>
> Can development interests grow also according to another clarification approach?
>
> Does &((struct name *)NULL -> b) cause undefined behaviour in C11?
> https://stackoverflow.com/questions/26906621/does-struct-name-null-b-cause-undefined-behaviour-in-c11
>
>
>
> Would you prefer to omit a “sanity check” in the discussed function implementation?
>
This is irrelevant.
In C writing if (!p) and if (p == NULL) are always equivalent but weirdly
the NULL doesn't have to zero. It's part of the C FAQ.
https://c-faq.com/null/machexamp.html It's just a bit of fun trivia that
doesn't really matter unless you have a time machine. Even if you invented
a time machine, the code here would still be fine because we have a NULL
test before dereferencing the results of our pointer math.
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH v3 net 3/6] xsk: provide sufficient space in pool->tx_descs
From: Jason Xing @ 2026-07-16 9:29 UTC (permalink / raw)
To: Maciej Fijalkowski
Cc: netdev, bpf, magnus.karlsson, stfomichev, kuba, pabeni, horms,
bjorn
In-Reply-To: <20260714140722.111645-4-maciej.fijalkowski@intel.com>
On Tue, Jul 14, 2026 at 4:08 PM Maciej Fijalkowski
<maciej.fijalkowski@intel.com> wrote:
>
> The temporary Tx descriptor array in an XSK buffer pool is currently
> sized from the Tx ring of the socket that creates the pool.
>
> This is insufficient for shared-UMEM Tx. A later socket may have a
> larger Tx ring and submit a valid multi-buffer packet containing more
> descriptors than the first socket's ring, while still remaining within
> the device's xdp_zc_max_segs limit.
>
> A packet-framed batch parser bounded by the temporary array cannot reach
> the end-of-packet descriptor in that case. It leaves the packet on the
> Tx ring and encounters the same packet on every subsequent attempt,
> stalling Tx processing for that socket.
>
> Size the temporary descriptor array to the larger of the first Tx ring
> and the device's xdp_zc_max_segs capability. This keeps the array large
> enough to inspect one maximum-sized valid packet. Larger shared Tx rings
> do not require further resizing, as they can be processed over multiple
> batches.
>
> Following commit will actually address the data path side.
>
> Fixes: d5581966040f ("xsk: support ZC Tx multi-buffer in batch API")
> Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Reviewed-by: Jason Xing <kerneljasonxing@gmail.com>
I noticed there is one interesting comment[1] from sashiko, which
actually I think is valid. Probably we don't need to do it in the fix,
but we might need to target -next branch. Now the tx_descs is limited
by the first tx ring.
[1]:
"...since tx_descs is not
reallocated if it already exists, the array size is permanently frozen
to the first socket's size.
Should the array be resized to accommodate the largest shared ring, or
should the batch readers be updated to cap their reads to the array's
actual size?"
Thanks,
Jason
> ---
> include/net/xsk_buff_pool.h | 6 ++++--
> net/xdp/xsk.c | 10 +++++++---
> net/xdp/xsk_buff_pool.c | 12 ++++++++----
> 3 files changed, 19 insertions(+), 9 deletions(-)
>
> diff --git a/include/net/xsk_buff_pool.h b/include/net/xsk_buff_pool.h
> index ccb3b350001f..f5e737a83055 100644
> --- a/include/net/xsk_buff_pool.h
> +++ b/include/net/xsk_buff_pool.h
> @@ -102,12 +102,14 @@ struct xsk_buff_pool {
>
> /* AF_XDP core. */
> struct xsk_buff_pool *xp_create_and_assign_umem(struct xdp_sock *xs,
> - struct xdp_umem *umem);
> + struct xdp_umem *umem,
> + u32 max_segs);
> int xp_assign_dev(struct xsk_buff_pool *pool, struct net_device *dev,
> u16 queue_id, u16 flags);
> int xp_assign_dev_shared(struct xsk_buff_pool *pool, struct xdp_sock *umem_xs,
> struct net_device *dev, u16 queue_id);
> -int xp_alloc_tx_descs(struct xsk_buff_pool *pool, struct xdp_sock *xs);
> +int xp_alloc_tx_descs(struct xsk_buff_pool *pool, struct xdp_sock *xs,
> + u32 max_segs);
> void xp_destroy(struct xsk_buff_pool *pool);
> void xp_get_pool(struct xsk_buff_pool *pool);
> bool xp_put_pool(struct xsk_buff_pool *pool);
> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index 43791647cf18..385a3f4a1b32 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c
> @@ -1525,7 +1525,8 @@ static int xsk_bind(struct socket *sock, struct sockaddr_unsized *addr, int addr
> * and/or device.
> */
> xs->pool = xp_create_and_assign_umem(xs,
> - umem_xs->umem);
> + umem_xs->umem,
> + dev->xdp_zc_max_segs);
> if (!xs->pool) {
> err = -ENOMEM;
> sockfd_put(sock);
> @@ -1557,7 +1558,8 @@ static int xsk_bind(struct socket *sock, struct sockaddr_unsized *addr, int addr
> * utilizes
> */
> if (xs->tx && !xs->pool->tx_descs) {
> - err = xp_alloc_tx_descs(xs->pool, xs);
> + err = xp_alloc_tx_descs(xs->pool, xs,
> + dev->xdp_zc_max_segs);
> if (err) {
> xp_put_pool(xs->pool);
> xs->pool = NULL;
> @@ -1575,7 +1577,9 @@ static int xsk_bind(struct socket *sock, struct sockaddr_unsized *addr, int addr
> goto out_unlock;
> } else {
> /* This xsk has its own umem. */
> - xs->pool = xp_create_and_assign_umem(xs, xs->umem);
> + xs->pool = xp_create_and_assign_umem(xs, xs->umem,
> + dev->xdp_zc_max_segs);
> +
> if (!xs->pool) {
> err = -ENOMEM;
> goto out_unlock;
> diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c
> index 1f28a9641571..12c9fb29af05 100644
> --- a/net/xdp/xsk_buff_pool.c
> +++ b/net/xdp/xsk_buff_pool.c
> @@ -42,9 +42,12 @@ void xp_destroy(struct xsk_buff_pool *pool)
> kvfree(pool);
> }
>
> -int xp_alloc_tx_descs(struct xsk_buff_pool *pool, struct xdp_sock *xs)
> +int xp_alloc_tx_descs(struct xsk_buff_pool *pool, struct xdp_sock *xs,
> + u32 max_segs)
> {
> - pool->tx_descs = kvzalloc_objs(*pool->tx_descs, xs->tx->nentries);
> + u32 nentries = max(xs->tx->nentries, max_segs);
> +
> + pool->tx_descs = kvzalloc_objs(*pool->tx_descs, nentries);
> if (!pool->tx_descs)
> return -ENOMEM;
>
> @@ -52,7 +55,8 @@ int xp_alloc_tx_descs(struct xsk_buff_pool *pool, struct xdp_sock *xs)
> }
>
> struct xsk_buff_pool *xp_create_and_assign_umem(struct xdp_sock *xs,
> - struct xdp_umem *umem)
> + struct xdp_umem *umem,
> + u32 max_segs)
> {
> bool unaligned = umem->flags & XDP_UMEM_UNALIGNED_CHUNK_FLAG;
> struct xsk_buff_pool *pool;
> @@ -69,7 +73,7 @@ struct xsk_buff_pool *xp_create_and_assign_umem(struct xdp_sock *xs,
> goto out;
>
> if (xs->tx)
> - if (xp_alloc_tx_descs(pool, xs))
> + if (xp_alloc_tx_descs(pool, xs, max_segs))
> goto out;
>
> pool->chunk_mask = ~((u64)umem->chunk_size - 1);
> --
> 2.43.0
>
^ permalink raw reply
* RE: [PATCH net v3 2/2] tipc: fix NULL deref in tipc_named_node_up() on empty publication list
From: Tung Quang Nguyen @ 2026-07-16 9:28 UTC (permalink / raw)
To: Weiming Shi
Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, Xiang Mei, linux-kernel@vger.kernel.org, Jon Maloy,
netdev@vger.kernel.org, tipc-discussion@lists.sourceforge.net
In-Reply-To: <20260714174110.1571033-3-bestswngs@gmail.com>
>named_distribute() ends by stamping the last_bulk flag on the tail skb via
>buf_msg(skb_peek_tail(list)). When the publication list is empty no skb is
>enqueued, skb_peek_tail() returns NULL, and buf_msg(NULL) is dereferenced.
>
>tipc_named_node_up() runs this on &nt->cluster_scope. With a node-id
>configuration cluster_scope is populated only later by tipc_net_finalize(), so a
>peer link that comes up first reaches named_distribute() with an empty list. It
>is reachable by an unprivileged user (TIPC genl ops use
>GENL_UNS_ADMIN_PERM) over a UDP bearer in a user+net namespace:
>
> KASAN: null-ptr-deref in range [0x00000000000000d8-0x00000000000000df]
> RIP: 0010:tipc_named_node_up (net/tipc/name_distr.c:196)
> tipc_named_node_up (net/tipc/name_distr.c:196 net/tipc/name_distr.c:221)
> tipc_node_write_unlock (net/tipc/node.c:428)
> tipc_rcv (net/tipc/node.c:2185)
> tipc_udp_recv (net/tipc/udp_media.c:392) Kernel panic - not syncing: Fatal
>exception in interrupt
>
>The peer holds back this node's later name updates until it sees a bulk with the
>last_bulk flag, so simply skipping the empty bulk would stall it.
>Emit an item-less bulk when the list is empty, and break out of the build loop
>on allocation failure instead of returning, so the last_bulk flag is applied to the
>last queued skb.
>
>Fixes: cad2929dc432 ("tipc: update a binding service via broadcast")
>Reported-by: Xiang Mei <xmei5@asu.edu>
>Assisted-by: Claude:claude-opus-4-8
>Signed-off-by: Weiming Shi <bestswngs@gmail.com>
>---
> net/tipc/name_distr.c | 16 +++++++++++++++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
>diff --git a/net/tipc/name_distr.c b/net/tipc/name_distr.c index
>ba4f4906e13b..dbcfa965de34 100644
>--- a/net/tipc/name_distr.c
>+++ b/net/tipc/name_distr.c
>@@ -165,7 +165,7 @@ static void named_distribute(struct net *net, struct
>sk_buff_head *list,
> dnode);
> if (!skb) {
> pr_warn("Bulk publication failure\n");
>- return;
>+ break;
> }
> hdr = buf_msg(skb);
> msg_set_bc_ack_invalid(hdr, true);
>@@ -192,6 +192,20 @@ static void named_distribute(struct net *net, struct
>sk_buff_head *list,
> skb_trim(skb, INT_H_SIZE + (msg_dsz - msg_rem));
> __skb_queue_tail(list, skb);
> }
>+
>+ if (skb_queue_empty(list)) {
>+ skb = named_prepare_buf(net, PUBLICATION, 0, dnode);
>+ if (!skb) {
>+ pr_warn("Bulk publication failure\n");
>+ return;
>+ }
This approach is wrong because:
1. When 'list' is empty, it is caused by memory allocation failure before. So, it is likely that 'skb' could be NULL again because of memory allocation failure.
2. Even if 'skb' is not NULL, allocation of non-data (zero-in-size) message will break the receiving peer when it handles this message.
>+ hdr = buf_msg(skb);
>+ msg_set_bc_ack_invalid(hdr, true);
>+ msg_set_bulk(hdr);
>+ msg_set_non_legacy(hdr);
>+ __skb_queue_tail(list, skb);
>+ }
>+
> hdr = buf_msg(skb_peek_tail(list));
> msg_set_last_bulk(hdr);
> msg_set_named_seqno(hdr, seqno);
>--
>2.43.0
^ permalink raw reply
* Re: [for-next v4 0/5] ionic: RDMA completion timestamping support
From: Leon Romanovsky @ 2026-07-16 9:19 UTC (permalink / raw)
To: Abhijit Gangurde
Cc: jgg, kuba, davem, allen.hubbe, andrew+netdev, brett.creeley,
edumazet, pabeni, nikhil.agarwal, linux-rdma, netdev,
linux-kernel, dwmw2
In-Reply-To: <7b5f02bb-26db-175e-34cc-40318b0752b2@amd.com>
On Thu, Jul 16, 2026 at 02:26:22PM +0530, Abhijit Gangurde wrote:
> Thanks Leon. This is already addressed in https://patchwork.kernel.org/project/linux-rdma/patch/20260617132605.1888205-3-abhijit.gangurde@amd.com/
> which was sent after this series. Happy to rebase if needed.
You need to rebase this, as we moved ib_respond_empty_udata()
from the end of the function to the beginning.
Also, please send this as a separate patch or series.
Thanks.
>
>
> On 7/16/26 13:31, Leon Romanovsky wrote:
> > On Wed, Jul 15, 2026 at 06:38:06PM +0530, Abhijit Gangurde wrote:
> > > Hi Jason, Jakub,
> > >
> > > Gentle ping — if there are no further concerns, could this series be merged
> > > through the rdma tree?
> > Jason requested converting any driver that extends udata to
> > use uverbs_robust_udata.
> >
> > https://lore.kernel.org/linux-rdma/13-v3-bd56dd443069+49-bnxt_re_uapi_jgg@nvidia.com/
> >
> > Thanks
> >
> > > Thanks,
> > > Abhijit
> > >
> > > On 6/10/26 21:12, Abhijit Gangurde wrote:
> > > > Hi,
> > > >
> > > > This series adds RDMA completion timestamp support for ionic.
> > > >
> > > > It enables PHC registration for RDMA timestamp capability, exposes a PHC
> > > > state page for safe user-space reads, maps that PHC state through RDMA
> > > > ucontext mmap, and extends the RDMA CQE format to carry completion
> > > > timestamps.
> > > >
> > > > With this, user space can read completion timestamps and convert them to
> > > > wall time with low overhead.
> > > >
> > > > Provider's PR: https://github.com/linux-rdma/rdma-core/pull/1724
> > > >
> > > > v4:
> > > > - Added alias mapping of mlx5_ib_clock_info to ib_uverbs_clock_info
> > > > v3:
> > > > - Renamed ib_uverbs_phc_state to ib_uverbs_clock_info
> > > > - Moved mlx5 to use the common clock info structure
> > > > - Addressed review feedback from Sashiko
> > > > - https://lore.kernel.org/linux-rdma/20260606050003.3648306-1-abhijit.gangurde@amd.com/
> > > > v2:
> > > > - changed ionic_phc_state to ib_uverbs_phc_state and moved it under
> > > > ib_user_verbs.h
> > > > - https://lore.kernel.org/linux-rdma/20260512092623.1157199-1-abhijit.gangurde@amd.com/
> > > > v1:
> > > > - https://lore.kernel.org/all/20260401102501.3395305-1-abhijit.gangurde@amd.com/
> > > >
> > > > Abhijit Gangurde (5):
> > > > net: ionic: register PHC for rdma timestamping
> > > > net: ionic: Add PHC state page for user space access
> > > > RDMA/ionic: map PHC state into user space
> > > > RDMA/ionic: add completion timestamp to CQE format
> > > > RDMA/mlx5: move mlx5 clock info to common struct ib_uverbs_clock_info
> > > >
> > > > .../infiniband/hw/ionic/ionic_controlpath.c | 34 ++++++++++
> > > > drivers/infiniband/hw/ionic/ionic_datapath.c | 43 ++++++-------
> > > > drivers/infiniband/hw/ionic/ionic_fw.h | 12 +++-
> > > > drivers/infiniband/hw/ionic/ionic_ibdev.h | 2 +
> > > > drivers/infiniband/hw/ionic/ionic_lif_cfg.c | 2 +
> > > > drivers/infiniband/hw/ionic/ionic_lif_cfg.h | 1 +
> > > > .../ethernet/pensando/ionic/ionic_ethtool.c | 12 ++--
> > > > .../net/ethernet/pensando/ionic/ionic_if.h | 1 +
> > > > .../net/ethernet/pensando/ionic/ionic_lif.c | 5 +-
> > > > .../net/ethernet/pensando/ionic/ionic_lif.h | 3 +-
> > > > .../net/ethernet/pensando/ionic/ionic_phc.c | 63 ++++++++++++++++---
> > > > include/uapi/rdma/ib_user_verbs.h | 33 ++++++++++
> > > > include/uapi/rdma/ionic-abi.h | 1 +
> > > > include/uapi/rdma/mlx5-abi.h | 15 ++---
> > > > 14 files changed, 179 insertions(+), 48 deletions(-)
> > > >
^ permalink raw reply
* Re: [PATCH net v5] rtase: Workaround for TX hang caused by hardware packet parsing
From: Simon Horman @ 2026-07-16 9:17 UTC (permalink / raw)
To: Justin Lai
Cc: kuba, davem, edumazet, pabeni, andrew+netdev, linux-kernel,
netdev, stable, richardcochran, david.laight.linux,
aleksander.lobakin, pkshih, larry.chiu
In-Reply-To: <20260709103456.83789-1-justinlai0215@realtek.com>
On Thu, Jul 09, 2026 at 06:34:56PM +0800, Justin Lai wrote:
> The hardware performs packet parsing before packet transmission.
> Parsing incomplete IPv4, IPv6, TCP, or UDP headers may trigger a TX
> hang because the hardware parser expects additional protocol header
> data that is not present in the packet.
>
> The hardware performs additional PTP parsing on UDP packets identified
> by destination ports 319/320 at the expected UDP destination port
> offset.
>
> If such a packet has transport data smaller than RTASE_MIN_PAD_LEN,
> the hardware parser expects additional packet data and may trigger a
> TX hang.
>
> To avoid these hardware issues, the driver applies the following
> workarounds.
>
> Drop malformed packets that may trigger this hardware issue before
> transmission.
>
> For IPv4 non-initial fragments, the hardware does not check the
> fragment offset before parsing the expected transport header location.
> As a result, these packets are still subject to transport header
> parsing even though they do not contain a transport header. If the
> transport data is shorter than the minimum transport header required
> by the hardware parser, pad the transport data to the minimum
> transport header length required by the hardware parser. Packets that
> also match the hardware PTP parsing conditions continue to follow the
> corresponding workaround.
>
> For IPv6 fragmented packets, neither of the above hardware issues
> occurs because the hardware only continues packet parsing when the
> IPv6 Base Header Next Header field directly indicates UDP. Packets
> carrying a Fragment Header do not continue through the subsequent
> packet parsing stages.
>
> For packets identified for hardware PTP parsing, pad the transport
> data so it reaches RTASE_MIN_PAD_LEN before transmission.
>
> Fixes: d6e882b89fdf ("rtase: Implement .ndo_start_xmit function")
> Cc: stable@vger.kernel.org
> Signed-off-by: Justin Lai <justinlai0215@realtek.com>
Reviewed-by: Simon Horman <horms@kernel.org>
FTR, the AI-generated reviews of this patch on both sashiko.dev and
https://netdev-ai.bots.linux.dev/sashiko/ flag issues. However, I do
not believe they should impede progress of this patch.
^ permalink raw reply
* [PATCH] sctp: don't free the ASCONF's own transport in DEL-IP processing
From: =?gb18030?B?1uzIuMC2vvxBSUdCb3TTys/k?= @ 2026-07-16 9:12 UTC (permalink / raw)
To: netdev
Cc: Marcelo Ricardo Leitner, Xin Long, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, linux-sctp,
linux-kernel, Jun Yang, stable
sctp_process_asconf() caches the transport the ASCONF chunk is processed
against in asconf->transport (== chunk->transport, set once in sctp_rcv()).
For an ASCONF located through its Address Parameter by
__sctp_rcv_asconf_lookup(), that cached transport corresponds to the
Address Parameter, which need not be the packet's source address.
sctp_process_asconf_param() rejects a DEL-IP for the packet source address
(ADDIP D8, SCTP_ERROR_DEL_SRC_IP), but nothing protects asconf->transport.
A single ASCONF can therefore carry, in order:
[Address Parameter L] [DEL-IP L] [DEL-IP 0.0.0.0]
where L differs from the source. The DEL-IP for L passes the D8 check and
calls sctp_assoc_rm_peer() on the transport that asconf->transport still
points at, freeing it (RCU-deferred). The following wildcard DEL-IP then
reuses the now-dangling asconf->transport in sctp_assoc_set_primary() and
sctp_assoc_del_nonprimary_peers(): set_primary() dereferences the freed
transport (->ipaddr, ->state) and plants the dangling pointer into
asoc->peer.primary_path / active_path, and del_nonprimary_peers(), keeping
only the pointer that is no longer on the list, removes every real
transport, leaving the association with a transport_count of 0 and
primary_path/active_path pointing at freed memory.
Reject a DEL-IP that targets the transport the ASCONF is being processed
against, mirroring the existing source-address guard, so the wildcard
branch can never reuse a freed transport.
Fixes: 42e30bf3463c ("[SCTP]: Handle the wildcard ADD-IP Address parameter")
Cc: stable@kernel.org
Signed-off-by: Jun Yang <junvyyang@tencent.com>
Acked-by: Xin Long <lucien.xin@gmail.com>
---
net/sctp/sm_make_chunk.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index 8adac9e0cd66..b14251214896 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -3153,6 +3153,12 @@ static __be16 sctp_process_asconf_param(struct sctp_association *asoc,
if (!peer)
return SCTP_ERROR_DNS_FAILED;
+ /* Don't free asconf->transport; a later wildcard DEL-IP
+ * parameter reuses it.
+ */
+ if (peer == asconf->transport)
+ return SCTP_ERROR_DEL_SRC_IP;
+
sctp_assoc_rm_peer(asoc, peer);
break;
case SCTP_PARAM_SET_PRIMARY:
--
2.55.0
^ permalink raw reply related
* Re: [PATCH rdma-next v3] RDMA/mlx5: quiesce CQ polling before device shutdown on reboot
From: Chenguang Zhao @ 2026-07-16 9:03 UTC (permalink / raw)
To: Leon Romanovsky
Cc: jgg, andrew+netdev, davem, edumazet, kuba, pabeni, linux-rdma,
netdev, tariqt, mbloch, dtatulea, shayd, moshe, Chenguang Zhao
In-Reply-To: <20260716084211.GC70906@unreal>
Hi, Leon
reboot -f skips orderly shutdown and goes directly to:
kernel_restart_prepare() -> device_shutdown() -> mlx5 shutdown
Upper layers may still hold live CQs, while ib-comp-wq keeps
polling — a use-after-free race.
Normal reboot usually works because userspace has already
torn down RDMA and called ib_free_cq().
Thanks
在 2026/7/16 16:42, Leon Romanovsky 写道:
> On Wed, Jul 15, 2026 at 04:23:07PM +0800, Chenguang Zhao wrote:
>> From: Chenguang Zhao <zhaochenguang@kylinos.cn>
>>
>> On reboot -f with NFS over RDMA, mlx5 shutdown can tear the device
>> down while ib-comp-wq still polls live CQs, leading to UAF in
>> wr_cqe->done().
>>
>> Mark the device shutting down before teardown, flush completion
>> workqueues so in-flight pollers observe the flag, skip SYS_ERROR
>> completion delivery, and make poll/arm CQ a no-op under the CQ lock
>> while shutting down.
>>
>> Signed-off-by: Chenguang Zhao <zhaochenguang@kylinos.cn>
>> ---
>> changelog:
>> - Fix the race on MLX5_INTERFACE_STATE_SHUTTING_DOWN: set the
>> flag, then flush ib-comp / mlx5_ib event workqueues via an
>> mlx5_ib quiesce hook before fast_unload/teardown.
>> - Check shutting-down under cq->lock in mlx5_ib_poll_cq/arm_cq.
>> - Export ib_comp_wq and ib_comp_unbound_wq so modular mlx5_ib
>> can flush them.
>>
>> v2:
>> https://lore.kernel.org/all/20260714075558.1420384-1-chenguang.zhao@linux.dev/
>>
>> v1:
>> https://lore.kernel.org/all/20260702073422.279820-1-chenguang.zhao@linux.dev/
>>
>> drivers/infiniband/core/device.c | 2 ++
>> drivers/infiniband/hw/mlx5/cq.c | 11 ++++++++++
>> drivers/infiniband/hw/mlx5/main.c | 20 +++++++++++++++++++
>> .../net/ethernet/mellanox/mlx5/core/health.c | 3 +++
>> .../net/ethernet/mellanox/mlx5/core/main.c | 10 ++++++++++
>> .../mellanox/mlx5/core/sf/dev/driver.c | 3 +++
>> include/linux/mlx5/driver.h | 11 ++++++++++
>> 7 files changed, 60 insertions(+)
> <...>
>
>> +static void mlx5_ib_shutdown_quiesce(void)
>> +{
>> + flush_workqueue(ib_comp_wq);
>> + flush_workqueue(ib_comp_unbound_wq);
>> + flush_workqueue(mlx5_ib_event_wq);
>> +}
> These workqueues are shared by all IB drivers and the core. Drivers
> must not flush or destroy them.
>
> Why this is not FW issue?
>
> Thanks
^ permalink raw reply
* [PATCH 6.1] ipv4: account for fraggap on the paged allocation path
From: Alexander Martyniuk @ 2026-07-16 9:01 UTC (permalink / raw)
To: stable, Greg Kroah-Hartman
Cc: Alexander Martyniuk, David S. Miller, Hideaki YOSHIFUJI,
David Ahern, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Pavel Begunkov, netdev, linux-kernel, lvc-project, Jungwoo Lee,
Wongi Lee, Ido Schimmel
From: Wongi Lee <qw3rtyp0@gmail.com>
commit eca856950f7cb1a221e02b99d758409f2c5cec42 upstream.
In __ip_append_data(), when the paged-allocation branch is taken,
alloclen and pagedlen are computed as
alloclen = fragheaderlen + transhdrlen;
pagedlen = datalen - transhdrlen;
datalen already includes fraggap, but the fraggap bytes carried over
from the previous skb are copied into the new skb's linear area at
offset transhdrlen by the subsequent skb_copy_and_csum_bits(). The
linear area is therefore undersized by fraggap bytes while pagedlen is
overstated by the same amount.
The non-paged branch sets alloclen to fraglen, which already accounts
for fraggap because datalen does. Bring the paged branch in line by
adding fraggap to alloclen and subtracting it from pagedlen.
After this adjustment, copy no longer collapses to -fraggap on the
paged path, so remove the stale comment describing that old arithmetic.
Fixes: 8eb77cc73977 ("ipv4: avoid partial copy for zc")
Signed-off-by: Jungwoo Lee <jwlee2217@gmail.com>
Signed-off-by: Wongi Lee <qw3rtyp0@gmail.com>
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
Link: https://patch.msgid.link/ajFR1eLAIs42TN3g@DESKTOP-19IMU7U.localdomain
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Alexander Martyniuk <alexevgmart@gmail.com>
---
net/ipv4/ip_output.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index f5a2ad2b2dbd..778214137e4a 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -1117,8 +1117,8 @@ static int __ip_append_data(struct sock *sk,
!(rt->dst.dev->features & NETIF_F_SG)))
alloclen = fraglen;
else {
- alloclen = fragheaderlen + transhdrlen;
- pagedlen = datalen - transhdrlen;
+ alloclen = fragheaderlen + transhdrlen + fraggap;
+ pagedlen = datalen - transhdrlen - fraggap;
}
alloclen += alloc_extra;
--
2.43.0
^ permalink raw reply related
* Re: [PATCH 1/2] net/mlx5e: kTLS: reject stale RX queue mapping on RX offload setup
From: Tariq Toukan @ 2026-07-16 9:02 UTC (permalink / raw)
To: Rishikesh Jethwani, netdev
Cc: john.fastabend, kuba, sd, davem, pabeni, edumazet, leon,
nils.juenemann
In-Reply-To: <16711470-33b2-4aca-917f-8a5c2a83738e@nvidia.com>
On 16/07/2026 11:35, Tariq Toukan wrote:
>
>
> On 15/07/2026 1:29, Rishikesh Jethwani wrote:
>> mlx5e_ktls_sk_get_rxq() treats only the -1 sentinel from
>> sk_rx_queue_get() as special and returns all other values unchanged.
>>
>> After 'ethtool -L <dev> combined N' reduces the number of channels, a
>> socket can retain an sk_rx_queue_mapping from the previous
>> configuration that is no longer valid for the current
>> priv->channels.num. If TLS RX offload is then enabled for that socket,
>> mlx5e_ktls_add_rx() uses the stale queue index and can access a
>> channel outside the current array.
>>
>> Preserve the existing -1 -> 0 fallback for sockets that do not yet
>> have a recorded RX queue, but reject queue indices that are outside
>> the current channel range and fail setup with -EINVAL instead. Wire
>> the failure through the existing err_create_tir unwind so resources
>> allocated earlier in mlx5e_ktls_add_rx() are released cleanly.
>>
>> This addresses stale queue mappings during RX offload setup. Existing
>> offloaded sockets whose channel disappears after reconfiguration are
>> handled separately in the resync path.
>>
>> Fixes: 1182f3659357 ("net/mlx5e: kTLS, Add kTLS RX HW offload support")
>> Link: https://lore.kernel.org/netdev/20260627210635.89769-1-
>> nils.juenemann@gmail.com/
>> Reported-by: Nils Juenemann <nils.juenemann@gmail.com>
>> Tested-by: Nils Juenemann <nils.juenemann@gmail.com>
>> Signed-off-by: Rishikesh Jethwani <rjethwani@purestorage.com>
>> ---
>
> Thanks for your patch.
>
>> .../ethernet/mellanox/mlx5/core/en_accel/ktls_rx.c | 13 ++++++++++---
>> 1 file changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/
>> ktls_rx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_rx.c
>> index bca45679e201..232e998a8f24 100644
>> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_rx.c
>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_rx.c
>> @@ -620,12 +620,15 @@ void mlx5e_ktls_handle_ctx_completion(struct
>> mlx5e_icosq_wqe_info *wi)
>> queue_work(rule->priv->tls->rx_wq, &rule->work);
>> }
>> -static int mlx5e_ktls_sk_get_rxq(struct sock *sk)
>> +static int mlx5e_ktls_sk_get_rxq(struct mlx5e_priv *priv, struct sock
>> *sk)
>> {
>> int rxq = sk_rx_queue_get(sk);
>> if (unlikely(rxq == -1))
>> - rxq = 0;
>> + return 0;
>> +
>> + if (unlikely(rxq >= priv->channels.num))
>> + return -EINVAL;
>> return rxq;
>> }
>> @@ -673,7 +676,11 @@ int mlx5e_ktls_add_rx(struct net_device *netdev,
>> struct sock *sk,
>> INIT_LIST_HEAD(&priv_rx->list);
>> spin_lock_init(&priv_rx->lock);
>> - rxq = mlx5e_ktls_sk_get_rxq(sk);
>> + rxq = mlx5e_ktls_sk_get_rxq(priv, sk);
>> + if (unlikely(rxq < 0)) {
>> + err = rxq;
>> + goto err_create_tir;
>> + }
>
> This is not bullet proof.
> Here you just shorten the interval and reduce the probability of the bug.
>
> As this flow is not protected by the state_lock, it is still possible to
> have the num of channels changing after your read above.
>
> IMO, this can't be resolved without holding the mutex here.
> I'm doing further research to come up with the proper solution.
>
Please note we were not CCed on this email.
Let's make sure we're not missed on future ones.
^ permalink raw reply
* Re: [PATCH v4 4/5] vhost: synchronize with RCU readers when freeing workers
From: Stefano Garzarella @ 2026-07-16 8:57 UTC (permalink / raw)
To: Andrey Drobyshev
Cc: linux-kernel, kvm, virtualization, netdev, mst, stefanha,
dongli.zhang, maciej.szmigiero, bchaney, mark.kanda, ptikhomirov,
den
In-Reply-To: <20260714151638.143019-5-andrey.drobyshev@virtuozzo.com>
On Tue, Jul 14, 2026 at 06:16:37PM +0300, Andrey Drobyshev wrote:
>vhost_vq_work_queue() only holds the RCU read lock while it dereferences
>vq->worker and queues work on it. vhost_workers_free() however clears
>the vq->worker pointers and immediately frees the workers, without
>waiting for a grace period. A caller that fetched the worker right
>before the pointer was cleared can therefore still be queueing work on
>it while it is freed. And even when the queueing itself wins the race,
>the work is never run, so its VHOST_WORK_QUEUED bit stays set and all
>future attempts to queue it are silently skipped.
>
>None of the current callers can actually hit this: net and scsi stop
>their virtqueues before the workers are freed, and vsock unhashes the
>device and does synchronize_rcu() of its own in vhost_vsock_dev_release()
>before the workers go away. But the upcoming VHOST_RESET_OWNER support
>in vhost-vsock keeps the device hashed while its workers are freed, so
>the lockless send/cancel paths become able to race with the teardown.
>
>Close this the way vhost_worker_killed() already does: clear the
>vq->worker pointers, wait for a grace period, run whatever the last
>readers may have queued, and only then free the workers. The
>synchronize_rcu() is skipped if the device has no workers, so cleanup of
>devices which never got an owner stays cheap.
>
Do we need a Fixes tag for this?
Thanks for pointing out that the issue wasn't occurring, but I think we
should add it because it's a sneaky problem we discovered by chance.
IMO the code should already have `synchronize_rcu()` after
`rcu_assign_pointer()` loop.
@Michael, what do you think?
>Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
>Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
>---
> drivers/vhost/vhost.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
>diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
>index 4c525b3e16ea..0d1414d40f4e 100644
>--- a/drivers/vhost/vhost.c
>+++ b/drivers/vhost/vhost.c
>@@ -729,6 +729,21 @@ static void vhost_workers_free(struct vhost_dev *dev)
>
> for (i = 0; i < dev->nvqs; i++)
> rcu_assign_pointer(dev->vqs[i]->worker, NULL);
>+
>+ /*
>+ * vhost_vq_work_queue() reads vq->worker under rcu_read_lock(), so a
>+ * caller that fetched a worker before we cleared the pointers above
>+ * may still be about to queue work on it. Wait for those RCU readers
>+ * to finish before freeing the worker, then run whatever they queued
>+ * so nothing is left with VHOST_WORK_QUEUED set. Mirrors
>+ * vhost_worker_killed().
>+ */
>+ if (!xa_empty(&dev->worker_xa)) {
>+ synchronize_rcu();
>+ xa_for_each(&dev->worker_xa, i, worker)
>+ vhost_run_work_list(worker);
>+ }
>+
Following sashiko review [1], I tried to undersand why we need this, but
TBH I'm really confused. That said, this seems wrong also because it
will work only with vhost_tasks, and not with kthreads.
IIUC vhost_worker_killed() will be called anyway when calling
vhost_worker_destroy(). For vhost_tasks, it will call
vhost_task_do_stop() that calls vhost_task_stop(). This sets
VHOST_TASK_FLAGS_STOP and wait the worker on vtsk->exited before freeing
stuff. The worker breaks the loop and calls vtsk->handle_sigkill() that
is exactly vhost_worker_killed() you mentioned we are mirroring here.
So, why we need this?
Should be enough to call synchronize_rcu() in any case after the
rcu_assign_pointer() loop?
Thanks,
Stefano
[1]
https://sashiko.dev/#/patchset/20260714151638.143019-1-andrey.drobyshev@virtuozzo.com?part=4
^ permalink raw reply
* Re: [for-next v4 0/5] ionic: RDMA completion timestamping support
From: Abhijit Gangurde @ 2026-07-16 8:56 UTC (permalink / raw)
To: Leon Romanovsky
Cc: jgg, kuba, davem, allen.hubbe, andrew+netdev, brett.creeley,
edumazet, pabeni, nikhil.agarwal, linux-rdma, netdev,
linux-kernel, dwmw2
In-Reply-To: <20260716080110.GA70906@unreal>
Thanks Leon. This is already addressed in
https://patchwork.kernel.org/project/linux-rdma/patch/20260617132605.1888205-3-abhijit.gangurde@amd.com/
which was sent after this series. Happy to rebase if needed.
On 7/16/26 13:31, Leon Romanovsky wrote:
> On Wed, Jul 15, 2026 at 06:38:06PM +0530, Abhijit Gangurde wrote:
>> Hi Jason, Jakub,
>>
>> Gentle ping — if there are no further concerns, could this series be merged
>> through the rdma tree?
> Jason requested converting any driver that extends udata to
> use uverbs_robust_udata.
>
> https://lore.kernel.org/linux-rdma/13-v3-bd56dd443069+49-bnxt_re_uapi_jgg@nvidia.com/
>
> Thanks
>
>> Thanks,
>> Abhijit
>>
>> On 6/10/26 21:12, Abhijit Gangurde wrote:
>>> Hi,
>>>
>>> This series adds RDMA completion timestamp support for ionic.
>>>
>>> It enables PHC registration for RDMA timestamp capability, exposes a PHC
>>> state page for safe user-space reads, maps that PHC state through RDMA
>>> ucontext mmap, and extends the RDMA CQE format to carry completion
>>> timestamps.
>>>
>>> With this, user space can read completion timestamps and convert them to
>>> wall time with low overhead.
>>>
>>> Provider's PR: https://github.com/linux-rdma/rdma-core/pull/1724
>>>
>>> v4:
>>> - Added alias mapping of mlx5_ib_clock_info to ib_uverbs_clock_info
>>> v3:
>>> - Renamed ib_uverbs_phc_state to ib_uverbs_clock_info
>>> - Moved mlx5 to use the common clock info structure
>>> - Addressed review feedback from Sashiko
>>> - https://lore.kernel.org/linux-rdma/20260606050003.3648306-1-abhijit.gangurde@amd.com/
>>> v2:
>>> - changed ionic_phc_state to ib_uverbs_phc_state and moved it under
>>> ib_user_verbs.h
>>> - https://lore.kernel.org/linux-rdma/20260512092623.1157199-1-abhijit.gangurde@amd.com/
>>> v1:
>>> - https://lore.kernel.org/all/20260401102501.3395305-1-abhijit.gangurde@amd.com/
>>>
>>> Abhijit Gangurde (5):
>>> net: ionic: register PHC for rdma timestamping
>>> net: ionic: Add PHC state page for user space access
>>> RDMA/ionic: map PHC state into user space
>>> RDMA/ionic: add completion timestamp to CQE format
>>> RDMA/mlx5: move mlx5 clock info to common struct ib_uverbs_clock_info
>>>
>>> .../infiniband/hw/ionic/ionic_controlpath.c | 34 ++++++++++
>>> drivers/infiniband/hw/ionic/ionic_datapath.c | 43 ++++++-------
>>> drivers/infiniband/hw/ionic/ionic_fw.h | 12 +++-
>>> drivers/infiniband/hw/ionic/ionic_ibdev.h | 2 +
>>> drivers/infiniband/hw/ionic/ionic_lif_cfg.c | 2 +
>>> drivers/infiniband/hw/ionic/ionic_lif_cfg.h | 1 +
>>> .../ethernet/pensando/ionic/ionic_ethtool.c | 12 ++--
>>> .../net/ethernet/pensando/ionic/ionic_if.h | 1 +
>>> .../net/ethernet/pensando/ionic/ionic_lif.c | 5 +-
>>> .../net/ethernet/pensando/ionic/ionic_lif.h | 3 +-
>>> .../net/ethernet/pensando/ionic/ionic_phc.c | 63 ++++++++++++++++---
>>> include/uapi/rdma/ib_user_verbs.h | 33 ++++++++++
>>> include/uapi/rdma/ionic-abi.h | 1 +
>>> include/uapi/rdma/mlx5-abi.h | 15 ++---
>>> 14 files changed, 179 insertions(+), 48 deletions(-)
>>>
^ permalink raw reply
* [PATCH net-next V7 4/4] devlink: Apply eswitch mode boot defaults
From: Mark Bloch @ 2026-07-16 8:48 UTC (permalink / raw)
To: Jiri Pirko, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman
Cc: Saeed Mahameed, Leon Romanovsky, Tariq Toukan, Andrew Lunn,
Jonathan Corbet, Shuah Khan, netdev, linux-rdma, linux-doc,
Mark Bloch, Jiri Pirko
In-Reply-To: <20260716084852.549909-1-mbloch@nvidia.com>
Apply parsed devlink_eswitch_mode= defaults after devlink registration
and after successful reload.
Mark the default as pending when a devlink instance is allocated. Before
devl_unlock() releases the instance lock, apply a pending default when
the instance is registered. Since the default is applied while the lock
is still held, userspace cannot race with it.
Clear the pending state before calling into the driver so the boot
default remains a one-shot operation even if the mode change fails.
For successful reloads that performed DRIVER_REINIT, devlink_reload()
already holds the devlink instance lock and the driver has completed
reload_up(). Clear the pending state and apply the default directly from
the reload path.
Treat an explicit userspace eswitch mode request as consuming the pending
default, and clear it when unregistering the devlink instance.
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
Signed-off-by: Mark Bloch <mbloch@nvidia.com>
---
net/devlink/core.c | 3 ++
net/devlink/default.c | 70 +++++++++++++++++++++++++++++++++++--
net/devlink/dev.c | 6 ++++
net/devlink/devl_internal.h | 5 +++
4 files changed, 82 insertions(+), 2 deletions(-)
diff --git a/net/devlink/core.c b/net/devlink/core.c
index d791a8e4317d..02e6d3ef32f8 100644
--- a/net/devlink/core.c
+++ b/net/devlink/core.c
@@ -317,6 +317,7 @@ EXPORT_SYMBOL_GPL(devl_trylock);
void devl_unlock(struct devlink *devlink)
{
+ devlink_default_eswitch_mode_apply_pending(devlink);
mutex_unlock(&devlink->lock);
}
EXPORT_SYMBOL_GPL(devl_unlock);
@@ -429,6 +430,7 @@ void devl_unregister(struct devlink *devlink)
ASSERT_DEVLINK_REGISTERED(devlink);
devl_assert_locked(devlink);
+ devlink_default_eswitch_mode_apply_pending_clear(devlink);
devlink_notify_unregister(devlink);
xa_clear_mark(&devlinks, devlink->index, DEVLINK_REGISTERED);
devlink_rel_put(devlink);
@@ -490,6 +492,7 @@ struct devlink *__devlink_alloc(const struct devlink_ops *ops, size_t priv_size,
INIT_LIST_HEAD(&devlink->trap_group_list);
INIT_LIST_HEAD(&devlink->trap_policer_list);
INIT_RCU_WORK(&devlink->rwork, devlink_release);
+ devlink_default_eswitch_mode_init(devlink);
lockdep_register_key(&devlink->lock_key);
mutex_init(&devlink->lock);
lockdep_set_class(&devlink->lock, &devlink->lock_key);
diff --git a/net/devlink/default.c b/net/devlink/default.c
index 9b15b7b23e00..b80e3d0399e1 100644
--- a/net/devlink/default.c
+++ b/net/devlink/default.c
@@ -10,6 +10,7 @@
static char *devlink_default_esw_mode_param;
static bool devlink_default_esw_mode_match_all;
+static bool devlink_default_esw_mode_enabled;
static enum devlink_eswitch_mode devlink_default_esw_mode;
static LIST_HEAD(devlink_default_esw_mode_nodes);
@@ -154,6 +155,7 @@ static void __init devlink_default_eswitch_mode_nodes_clear(void)
}
devlink_default_esw_mode_match_all = false;
+ devlink_default_esw_mode_enabled = false;
}
static int __init devlink_default_eswitch_mode_parse(char *str)
@@ -180,14 +182,78 @@ static int __init devlink_default_eswitch_mode_parse(char *str)
return err;
err = devlink_default_eswitch_mode_handles_parse(handles);
- if (err)
+ if (err) {
devlink_default_eswitch_mode_nodes_clear();
- else
+ } else {
devlink_default_esw_mode = esw_mode;
+ devlink_default_esw_mode_enabled = true;
+ }
return err;
}
+static bool devlink_default_eswitch_mode_match(struct devlink *devlink)
+{
+ const char *bus_name = devlink_bus_name(devlink);
+ const char *dev_name = devlink_dev_name(devlink);
+ struct devlink_default_esw_mode_node *node;
+
+ if (devlink_default_esw_mode_match_all)
+ return true;
+
+ node = devlink_default_eswitch_mode_node_find(bus_name, dev_name);
+ return !!node;
+}
+
+void devlink_default_eswitch_mode_apply_locked(struct devlink *devlink)
+{
+ const struct devlink_ops *ops = devlink->ops;
+ int err;
+
+ devl_assert_locked(devlink);
+
+ if (!devlink_default_eswitch_mode_match(devlink))
+ return;
+
+ if (!ops->eswitch_mode_set) {
+ if (!devlink_default_esw_mode_match_all)
+ devl_warn(devlink,
+ "devlink_eswitch_mode= selected this device but eswitch mode setting is not supported\n");
+ return;
+ }
+
+ err = devlink_eswitch_mode_set(devlink, devlink_default_esw_mode, NULL);
+ if (err)
+ devl_warn(devlink,
+ "Couldn't apply default eswitch mode, err %d\n",
+ err);
+}
+
+void devlink_default_eswitch_mode_apply_pending(struct devlink *devlink)
+{
+ devl_assert_locked(devlink);
+
+ if (!devlink->default_esw_mode_apply_pending ||
+ !__devl_is_registered(devlink))
+ return;
+
+ devlink->default_esw_mode_apply_pending = false;
+ devlink_default_eswitch_mode_apply_locked(devlink);
+}
+
+void devlink_default_eswitch_mode_init(struct devlink *devlink)
+{
+ devlink->default_esw_mode_apply_pending =
+ devlink_default_esw_mode_enabled;
+}
+
+void devlink_default_eswitch_mode_apply_pending_clear(struct devlink *devlink)
+{
+ devl_assert_locked(devlink);
+
+ devlink->default_esw_mode_apply_pending = false;
+}
+
static int __init devlink_default_eswitch_mode_setup(char *str)
{
devlink_default_esw_mode_param = str;
diff --git a/net/devlink/dev.c b/net/devlink/dev.c
index 119ef105d0a7..6a8d4e1100c2 100644
--- a/net/devlink/dev.c
+++ b/net/devlink/dev.c
@@ -478,6 +478,11 @@ int devlink_reload(struct devlink *devlink, struct net *dest_net,
return err;
WARN_ON(!(*actions_performed & BIT(action)));
+ if (*actions_performed & BIT(DEVLINK_RELOAD_ACTION_DRIVER_REINIT)) {
+ devlink_default_eswitch_mode_apply_pending_clear(devlink);
+ devlink_default_eswitch_mode_apply_locked(devlink);
+ }
+
/* Catch driver on updating the remote action within devlink reload */
WARN_ON(memcmp(remote_reload_stats, devlink->stats.remote_reload_stats,
sizeof(remote_reload_stats)));
@@ -731,6 +736,7 @@ int devlink_nl_eswitch_set_doit(struct sk_buff *skb, struct genl_info *info)
u16 mode;
if (info->attrs[DEVLINK_ATTR_ESWITCH_MODE]) {
+ devlink_default_eswitch_mode_apply_pending_clear(devlink);
mode = nla_get_u16(info->attrs[DEVLINK_ATTR_ESWITCH_MODE]);
err = devlink_eswitch_mode_set(devlink, mode, info->extack);
if (err)
diff --git a/net/devlink/devl_internal.h b/net/devlink/devl_internal.h
index 8fde867f2c14..8270f91c9e84 100644
--- a/net/devlink/devl_internal.h
+++ b/net/devlink/devl_internal.h
@@ -58,6 +58,7 @@ struct devlink {
struct mutex lock;
struct lock_class_key lock_key;
u8 reload_failed:1;
+ u8 default_esw_mode_apply_pending:1;
refcount_t refcount;
struct rcu_work rwork;
struct devlink_rel *rel;
@@ -73,6 +74,10 @@ struct devlink *__devlink_alloc(const struct devlink_ops *ops, size_t priv_size,
const struct device_driver *dev_driver);
int devlink_default_eswitch_mode_cmdline_init(void);
void devlink_default_eswitch_mode_cleanup(void);
+void devlink_default_eswitch_mode_init(struct devlink *devlink);
+void devlink_default_eswitch_mode_apply_locked(struct devlink *devlink);
+void devlink_default_eswitch_mode_apply_pending(struct devlink *devlink);
+void devlink_default_eswitch_mode_apply_pending_clear(struct devlink *devlink);
#define devl_warn(devlink, format, args...) \
do { \
--
2.43.0
^ permalink raw reply related
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