netdev - RE: [Intel-wired-lan] [PATCH iwl-net] idpf: cleanup remaining SKBs in PTP flows

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <SJ1PR11MB629799C0FA6803FD3904067C9B1FA@SJ1PR11MB6297.namprd11.prod.outlook.com>
Date: Thu, 25 Sep 2025 22:28:45 +0000
From: "Salin, Samuel" <samuel.salin@...el.com>
To: "Nadezhdin, Anton" <anton.nadezhdin@...el.com>, Paul Menzel
	<pmenzel@...gen.mpg.de>, "Olech, Milena" <milena.olech@...el.com>
CC: "intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>, "Nguyen, Anthony L"
	<anthony.l.nguyen@...el.com>, "richardcochran@...il.com"
	<richardcochran@...il.com>, "Loktionov, Aleksandr"
	<aleksandr.loktionov@...el.com>
Subject: RE: [Intel-wired-lan] [PATCH iwl-net] idpf: cleanup remaining SKBs in
 PTP flows



> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@...osl.org> On Behalf Of
> Nadezhdin, Anton
> Sent: Thursday, August 14, 2025 1:17 AM
> To: Paul Menzel <pmenzel@...gen.mpg.de>; Olech, Milena
> <milena.olech@...el.com>
> Cc: intel-wired-lan@...ts.osuosl.org; netdev@...r.kernel.org; Nguyen,
> Anthony L <anthony.l.nguyen@...el.com>; richardcochran@...il.com;
> Loktionov, Aleksandr <aleksandr.loktionov@...el.com>
> Subject: Re: [Intel-wired-lan] [PATCH iwl-net] idpf: cleanup remaining SKBs in
> PTP flows
> 
> On 8/14/2025 7:19 AM, Paul Menzel wrote:
> > Dear Anton, dear Milena,
> >
> >
> > Thank you for the patch.
> >
> > Am 13.08.25 um 19:33 schrieb Anton Nadezhdin:
> >> From: Milena Olech <milena.olech@...el.com>
> >>
> >> When the driver requests Tx timestamp value, one of the first steps
> >> is to clone SKB using skb_get. It increases the reference counter for
> >> that SKB to prevent unexpected freeing by another component.
> >> However, there may be a case where the index is requested, SKB is
> >> assigned and never consumed by PTP flows - for example due to reset
> >> during running PTP apps.
> >>
> >> Add a check in release timestamping function to verify if the SKB
> >> assigned to Tx timestamp latch was freed, and release remaining SKBs.
> >>
> >> Fixes: 4901e83a94ef ("idpf: add Tx timestamp capabilities
> >> negotiation")
> >> Signed-off-by: Milena Olech <milena.olech@...el.com>
> >> Signed-off-by: Anton Nadezhdin <anton.nadezhdin@...el.com>
> >> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@...el.com>
> >> ---
> >>   drivers/net/ethernet/intel/idpf/idpf_ptp.c          | 3 +++
> >>   drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c | 1 +
> >>   2 files changed, 4 insertions(+)
> >>
> >> diff --git a/drivers/net/ethernet/intel/idpf/idpf_ptp.c
> >> b/drivers/net/ ethernet/intel/idpf/idpf_ptp.c index
> >> ee21f2ff0cad..63a41e688733 100644
> >> --- a/drivers/net/ethernet/intel/idpf/idpf_ptp.c
> >> +++ b/drivers/net/ethernet/intel/idpf/idpf_ptp.c
> >> @@ -855,6 +855,9 @@ static void idpf_ptp_release_vport_tstamp(struct
> >> idpf_vport *vport)
> >>       head = &vport->tx_tstamp_caps->latches_in_use;
> >>       list_for_each_entry_safe(ptp_tx_tstamp, tmp, head, list_member)
> >> {
> >>           list_del(&ptp_tx_tstamp->list_member);
> >> +        if (ptp_tx_tstamp->skb)
> >> +            consume_skb(ptp_tx_tstamp->skb);
> >> +
> >>           kfree(ptp_tx_tstamp);
> >>       }
> >>   diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c b/
> >> drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c
> >> index 4f1fb0cefe51..688a6f4e0acc 100644
> >> --- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c
> >> +++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c
> >> @@ -517,6 +517,7 @@ idpf_ptp_get_tstamp_value(struct idpf_vport
> >> *vport,
> >>       shhwtstamps.hwtstamp = ns_to_ktime(tstamp);
> >>       skb_tstamp_tx(ptp_tx_tstamp->skb, &shhwtstamps);
> >>       consume_skb(ptp_tx_tstamp->skb);
> >> +    ptp_tx_tstamp->skb = NULL;
> >
> > This hunk is not clear to me from the commit message, and the whole diff.
> 
> So in idpf_ptp_request_ts the code is making copy of skb, normally when this
> copy is consumed in idpf_ptp_get_tstamp_value but in case of a reset the
> normal flow is interrupted and skb is still in hold.
> This patch release it during reset procedure.
> 
> >
> >>         list_add(&ptp_tx_tstamp->list_member,
> >>            &tx_tstamp_caps->latches_free);
> >
> > Kind regards,
> >
> > Paul

Tested-by: Samuel Salin <Samuel.salin@...el.com>