> -----Original Message----- > From: Naman Gulati <[email protected]> > Sent: Wednesday, October 14, 2026 5:14 PM > To: Tantilov, Emil S <[email protected]> > Cc: [email protected]; Olech, Milena <[email protected]>; > Ilichev, Konstantin <[email protected]>; Hay, Joshua A > <[email protected]>; [email protected]; Chittim, Madhu > <[email protected]> > Subject: Re: [PATCH iwl-net 1/2] idpf: fix use-after-free in the PTP auxiliary > worker > > Thanks! > > Reviewed-by: Naman Gulati <[email protected]> > > On Fri, Oct 9, 2026 at 2:43 PM Emil Tantilov <[email protected]> > wrote: > > > > idpf_ptp_update_cached_phctime() runs from the PTP auxiliary worker > > and walks the Rx queues of every vport to refresh > > rxq->cached_phc_time. There is a small window on down that can result > > in use-after-free, detected by > > KASAN: > > > > BUG: KASAN: slab-use-after-free in > idpf_ptp_update_cached_phctime+0x367/0x4a0 [idpf] > > Write of size 8 at addr ff11002146c2a4a0 by task ptp3/8084 > > ... > > idpf_ptp_update_cached_phctime+0x367/0x4a0 [idpf] > > idpf_ptp_do_aux_work+0x37/0x50 [idpf] > > ptp_aux_kworker+0x64/0xd0 > > kthread_worker_fn+0x289/0xa20 > > ... > > Allocated by task 8615 on cpu 5 at 812.655509s: > > idpf_vport_queues_alloc+0x9e5/0x2580 [idpf] > > idpf_vport_open.constprop.0+0x12c/0x10f0 [idpf] > > idpf_open+0x1bb/0x230 [idpf] > > __dev_open+0x2f5/0x7c0 > > ... > > Freed by task 8628 on cpu 10 at 814.925757s: > > idpf_rxq_group_rel.part.0+0xef/0x5a0 [idpf] > > idpf_vport_queues_rel+0xaf/0x140 [idpf] > > idpf_vport_stop+0x49e/0x850 [idpf] > > idpf_stop+0x6d/0xc0 [idpf] > > __dev_close_many+0x24c/0x650 > > > > The existing rxq_grps test does not help. It is evaluated before the > > pointer is read again, so the worker can equally run past the kfree() > > and dereference a NULL rxq_grps with a stale num_rxq_grp. > > > > Add a per vport rxq_grps_lock and use it to serialize every transition > > of > > rsrc->rxq_grps against the worker: > > > > - Teardown: idpf_rxq_group_rel() sets rsrc->rxq_grps to NULL under the > > lock before anything reachable through it is freed. The worker > > re-reads the pointer under the same lock, so it either sees a live > > array while the release path is still blocked on the lock, or sees > > NULL and skips that vport. The frees stay outside the lock. > > > > - Allocation: Build the rxq_grps array privately and publish it to > > rsrc->rxq_grps under the lock only once every group is complete. The > > freeing of the group contents moves to idpf_rxq_grps_free(), which > > takes the array explicitly so the allocation error path can unwind > > safely. > > > > - Soft reset: idpf_vport_stop() freed rxq_grps and cleared the live > > vport's copy under rxq_grps_lock. Drop the stale pointer in the clone > > so the memcpy() below does not republish the freed array to > > the PTP aux worker before idpf_vport_open() reallocates it. The lock > > sits past the offsetof(struct idpf_vport, link_up) boundary that > > idpf_initiate_soft_reset() stops its memcpy() at, next to the wait > > queues and mutexes that are already kept out of the copy, so a soft > > reset never byte-copies it. The data it guards, dflt_qv_rsrc, is > > inside that copied region, which is why the clone has to be cleared > > rather than relying on the lock alone. > > > > Also, fix the NULL check in idpf_ptp_update_phctime_rxq_grp(). The > > split queue case took the address of a member before testing it, which > > can never be NULL, so test the rxq_sets[] entry itself. > > > > Fixes: 1a49cf814fe1 ("idpf: add Tx timestamp flows") > > Reviewed-by: Madhu Chittim <[email protected]> > > Signed-off-by: Emil Tantilov <[email protected]> > > --- > > drivers/net/ethernet/intel/idpf/idpf.h | 5 ++ > > drivers/net/ethernet/intel/idpf/idpf_lib.c | 6 ++ > > drivers/net/ethernet/intel/idpf/idpf_ptp.c | 16 ++++- > > drivers/net/ethernet/intel/idpf/idpf_txrx.c | 65 +++++++++++++++------ > > 4 files changed, 71 insertions(+), 21 deletions(-) > > > > diff --git a/drivers/net/ethernet/intel/idpf/idpf.h > > b/drivers/net/ethernet/intel/idpf/idpf.h > > index 470bc23c844c..9d63feda1bc6 100644 > > --- a/drivers/net/ethernet/intel/idpf/idpf.h > > +++ b/drivers/net/ethernet/intel/idpf/idpf.h > > @@ -373,6 +373,8 @@ struct idpf_q_vec_rsrc { > > * @default_vport: Use this vport if one isn't specified > > * @crc_enable: Enable CRC insertion offload > > * @link_up: True if link is up > > + * @rxq_grps_lock: Serializes every transition of the > > @dflt_qv_rsrc.rxq_grps > > + * pointer of a vport against the PTP auxiliary worker > > * @tx_tstamp_caps: Capabilities negotiated for Tx timestamping > > * @tstamp_config: The Tx tstamp config > > * @tstamp_task: Tx timestamping task @@ -406,6 +408,9 @@ struct > > idpf_vport { > > bool crc_enable; > > bool link_up; > > > > + /* Must be past the point idpf_initiate_soft_reset() memcpy()s */ > > + spinlock_t rxq_grps_lock; > > + > > struct idpf_ptp_vport_tx_tstamp_caps *tx_tstamp_caps; > > struct kernel_hwtstamp_config tstamp_config; > > struct work_struct tstamp_task; diff --git > > a/drivers/net/ethernet/intel/idpf/idpf_lib.c > > b/drivers/net/ethernet/intel/idpf/idpf_lib.c > > index 827c795afcb6..a1c739cdd540 100644 > > --- a/drivers/net/ethernet/intel/idpf/idpf_lib.c > > +++ b/drivers/net/ethernet/intel/idpf/idpf_lib.c > > @@ -1276,6 +1276,7 @@ static struct idpf_vport *idpf_vport_alloc(struct > idpf_adapter *adapter, > > vport->compln_clean_budget = IDPF_TX_COMPLQ_CLEAN_BUDGET; > > vport->default_vport = adapter->num_alloc_vports < > > idpf_get_default_vports(adapter); > > + spin_lock_init(&vport->rxq_grps_lock); > > > > rsrc = &vport->dflt_qv_rsrc; > > rsrc->dev = &adapter->pdev->dev; @@ -2075,6 +2076,11 @@ int > > idpf_initiate_soft_reset(struct idpf_vport *vport, > > } else { > > set_bit(IDPF_VPORT_DEL_QUEUES, vport->flags); > > idpf_vport_stop(vport, false); > > + > > + /* Drop the stale pointer in the clone, as rxq_grps was > > freed > > + * by idpf_vport_stop(). > > + */ > > + new_rsrc->rxq_grps = NULL; > > } > > > > err = idpf_send_add_queues_msg(adapter, vport_config, > > new_rsrc, diff --git a/drivers/net/ethernet/intel/idpf/idpf_ptp.c > > b/drivers/net/ethernet/intel/idpf/idpf_ptp.c > > index 71fe8b2a8b4e..9b15973d2f7e 100644 > > --- a/drivers/net/ethernet/intel/idpf/idpf_ptp.c > > +++ b/drivers/net/ethernet/intel/idpf/idpf_ptp.c > > @@ -348,9 +348,11 @@ idpf_ptp_update_phctime_rxq_grp(const struct > idpf_rxq_group *grp, bool split, > > } > > } else { > > for (i = 0; i < grp->splitq.num_rxq_sets; i++) { > > + if (!grp->splitq.rxq_sets[i]) > > + continue; > > + > > rxq = &grp->splitq.rxq_sets[i]->rxq; > > - if (rxq) > > - WRITE_ONCE(rxq->cached_phc_time, systime); > > + WRITE_ONCE(rxq->cached_phc_time, systime); > > } > > } > > } > > @@ -387,10 +389,17 @@ static int > idpf_ptp_update_cached_phctime(struct idpf_adapter *adapter) > > struct idpf_q_vec_rsrc *rsrc; > > bool split; > > > > - if (!vport || !vport->dflt_qv_rsrc.rxq_grps) > > + if (!vport) > > continue; > > > > rsrc = &vport->dflt_qv_rsrc; > > + > > + spin_lock(&vport->rxq_grps_lock); > > + if (!rsrc->rxq_grps) { > > + spin_unlock(&vport->rxq_grps_lock); > > + continue; > > + } > > + > > split = idpf_is_queue_model_split(rsrc->rxq_model); > > > > for (u16 i = 0; i < rsrc->num_rxq_grp; i++) { @@ > > -398,6 +407,7 @@ static int idpf_ptp_update_cached_phctime(struct > > idpf_adapter *adapter) > > > > idpf_ptp_update_phctime_rxq_grp(grp, split, > > systime); > > } > > + spin_unlock(&vport->rxq_grps_lock); > > } > > > > return 0; > > diff --git a/drivers/net/ethernet/intel/idpf/idpf_txrx.c > > b/drivers/net/ethernet/intel/idpf/idpf_txrx.c > > index af3213eae393..96f79554a4d0 100644 > > --- a/drivers/net/ethernet/intel/idpf/idpf_txrx.c > > +++ b/drivers/net/ethernet/intel/idpf/idpf_txrx.c > > @@ -1358,16 +1358,15 @@ static void idpf_rxq_sw_queue_rel(struct > > idpf_rxq_group *rx_qgrp) } > > > > /** > > - * idpf_rxq_group_rel - Release all resources for rxq groups > > + * idpf_rxq_grps_free - Free an Rx queue group array and everything > > + in it > > * @rsrc: pointer to queue and vector resources > > + * @rxq_grps: group array to free, not reachable through @rsrc any > > + more > > */ > > -static void idpf_rxq_group_rel(struct idpf_q_vec_rsrc *rsrc) > > +static void idpf_rxq_grps_free(struct idpf_q_vec_rsrc *rsrc, > > + struct idpf_rxq_group *rxq_grps) > > { > > - if (!rsrc->rxq_grps) > > - return; > > - > > for (unsigned int i = 0; i < rsrc->num_rxq_grp; i++) { > > - struct idpf_rxq_group *rx_qgrp = &rsrc->rxq_grps[i]; > > + struct idpf_rxq_group *rx_qgrp = &rxq_grps[i]; > > u16 num_rxq; > > > > if (idpf_is_queue_model_split(rsrc->rxq_model)) { @@ > > -1388,19 +1387,40 @@ static void idpf_rxq_group_rel(struct > idpf_q_vec_rsrc *rsrc) > > } > > } > > } > > - kfree(rsrc->rxq_grps); > > + kfree(rxq_grps); > > +} > > + > > +/** > > + * idpf_rxq_group_rel - Release all resources for rxq groups > > + * @vport: vport the queue groups belong to > > + * @rsrc: pointer to queue and vector resources */ static void > > +idpf_rxq_group_rel(struct idpf_vport *vport, > > + struct idpf_q_vec_rsrc *rsrc) { > > + struct idpf_rxq_group *rxq_grps; > > + > > + if (!rsrc->rxq_grps) > > + return; > > + > > + spin_lock(&vport->rxq_grps_lock); > > + rxq_grps = rsrc->rxq_grps; > > rsrc->rxq_grps = NULL; > > + spin_unlock(&vport->rxq_grps_lock); > > + > > + idpf_rxq_grps_free(rsrc, rxq_grps); > > } > > > > /** > > * idpf_vport_queue_grp_rel_all - Release all queue groups > > - * @vport: vport to release queue groups for > > + * @vport: vport the queue groups belong to > > * @rsrc: pointer to queue and vector resources > > */ > > -static void idpf_vport_queue_grp_rel_all(struct idpf_q_vec_rsrc > > *rsrc) > > +static void idpf_vport_queue_grp_rel_all(struct idpf_vport *vport, > > + struct idpf_q_vec_rsrc *rsrc) > > { > > idpf_txq_group_rel(rsrc); > > - idpf_rxq_group_rel(rsrc); > > + idpf_rxq_group_rel(vport, rsrc); > > } > > > > /** > > @@ -1419,7 +1439,7 @@ void idpf_vport_queues_rel(struct idpf_vport > *vport, > > idpf_rx_desc_rel_all(rsrc); > > > > idpf_xdpsqs_put(vport); > > - idpf_vport_queue_grp_rel_all(rsrc); > > + idpf_vport_queue_grp_rel_all(vport, rsrc); > > > > kfree(vport->txqs); > > vport->txqs = NULL; > > @@ -1803,18 +1823,19 @@ static int idpf_rxq_group_alloc(struct > idpf_vport *vport, > > u16 num_rxq) { > > struct idpf_adapter *adapter = vport->adapter; > > + struct idpf_rxq_group *rxq_grps; > > bool hs, rsc; > > - int err = 0; > > + int err; > > > > - rsrc->rxq_grps = kzalloc_objs(struct idpf_rxq_group, rsrc- > >num_rxq_grp); > > - if (!rsrc->rxq_grps) > > + rxq_grps = kzalloc_objs(struct idpf_rxq_group, rsrc->num_rxq_grp); > > + if (!rxq_grps) > > return -ENOMEM; > > > > hs = idpf_vport_get_hsplit(vport) == > ETHTOOL_TCP_DATA_SPLIT_ENABLED; > > rsc = idpf_is_feature_ena(vport, NETIF_F_GRO_HW); > > > > for (unsigned int i = 0; i < rsrc->num_rxq_grp; i++) { > > - struct idpf_rxq_group *rx_qgrp = &rsrc->rxq_grps[i]; > > + struct idpf_rxq_group *rx_qgrp = &rxq_grps[i]; > > > > rx_qgrp->vport = vport; > > if (!idpf_is_queue_model_split(rsrc->rxq_model)) { @@ > > -1915,9 +1936,17 @@ static int idpf_rxq_group_alloc(struct idpf_vport > *vport, > > } > > } > > > > + /* Publish only once every group is built, the PTP aux worker reads > > + * the array as soon as it becomes visible. > > + */ > > + spin_lock(&vport->rxq_grps_lock); > > + rsrc->rxq_grps = rxq_grps; > > + spin_unlock(&vport->rxq_grps_lock); > > + > > + return 0; > > + > > err_alloc: > > - if (err) > > - idpf_rxq_group_rel(rsrc); > > + idpf_rxq_grps_free(rsrc, rxq_grps); > > > > return err; > > } > > @@ -1948,7 +1977,7 @@ static int idpf_vport_queue_grp_alloc_all(struct > idpf_vport *vport, > > return 0; > > > > err_out: > > - idpf_vport_queue_grp_rel_all(rsrc); > > + idpf_vport_queue_grp_rel_all(vport, rsrc); > > > > return err; > > } > > -- > > 2.37.3 > >
Tested-by: Samuel Salin <[email protected]>
