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: <5b196a7e-1992-439a-9407-30c1867bb9ee@intel.com>
Date: Thu, 14 Aug 2025 10:17:01 +0200
From: "Nadezhdin, Anton" <anton.nadezhdin@...el.com>
To: Paul Menzel <pmenzel@...gen.mpg.de>, Milena Olech <milena.olech@...el.com>
CC: <intel-wired-lan@...ts.osuosl.org>, <netdev@...r.kernel.org>,
	<anthony.l.nguyen@...el.com>, <richardcochran@...il.com>, Aleksandr Loktionov
	<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