netdev - Re: [PATCH net-next v3 2/2] net: flow_dissector: Parse PTP L2 packet header

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: <4e1c8783-6e98-0b73-cb28-eaabf7c51cfb@nvidia.com>
Date:   Tue, 12 Jan 2021 16:16:46 +0200
From:   Eran Ben Elisha <eranbe@...dia.com>
To:     Richard Cochran <richardcochran@...il.com>
CC:     "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>, <netdev@...r.kernel.org>,
        Tariq Toukan <tariqt@...dia.com>,
        Cong Wang <xiyou.wangcong@...il.com>,
        Eric Dumazet <edumazet@...gle.com>,
        Jiri Pirko <jiri@...dia.com>,
        Vladimir Oltean <vladimir.oltean@....com>,
        Jakub Sitnicki <jakub@...udflare.com>,
        Guillaume Nault <gnault@...hat.com>,
        Andrew Lunn <andrew@...n.ch>
Subject: Re: [PATCH net-next v3 2/2] net: flow_dissector: Parse PTP L2 packet
 header



On 1/12/2021 3:49 PM, Richard Cochran wrote:
> On Mon, Jan 11, 2021 at 08:17:48PM +0200, Eran Ben Elisha wrote:
>> Add support for parsing PTP L2 packet header. Such packet consists
>> of an L2 header (with ethertype of ETH_P_1588), PTP header, body
>> and an optional suffix.
>>
>> Signed-off-by: Eran Ben Elisha <eranbe@...dia.com>
>> Reviewed-by: Tariq Toukan <tariqt@...dia.com>
>> ---
>>   net/core/flow_dissector.c | 16 ++++++++++++++++
>>   1 file changed, 16 insertions(+)
>>
>> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
>> index 6f1adba6695f..fcaa223c7cdc 100644
>> --- a/net/core/flow_dissector.c
>> +++ b/net/core/flow_dissector.c
>> @@ -23,6 +23,7 @@
>>   #include <linux/if_ether.h>
>>   #include <linux/mpls.h>
>>   #include <linux/tcp.h>
>> +#include <linux/ptp_classify.h>
>>   #include <net/flow_dissector.h>
>>   #include <scsi/fc/fc_fcoe.h>
>>   #include <uapi/linux/batadv_packet.h>
>> @@ -1251,6 +1252,21 @@ bool __skb_flow_dissect(const struct net *net,
>>   						  &proto, &nhoff, hlen, flags);
>>   		break;
>>   
>> +	case htons(ETH_P_1588): {
>> +		struct ptp_header *hdr, _hdr;
>> +
>> +		hdr = __skb_header_pointer(skb, nhoff, sizeof(_hdr), data,
>> +					   hlen, &_hdr);
>> +		if (!hdr || (hlen - nhoff) < sizeof(_hdr)) {
> 
> I'm not really familiar with the flow dissector, but why check (hlen - nhoff) here?
> None of the other cases do that.  Doesn't skb_copy_bits() in
> __skb_header_pointer() already handle that?

You are right. I saw similar len validation at ETH_P_FCOE case. But now 
I see it does not go through __skb_header_pointer() flow.

Thanks.

> 
> Thanks,
> Richard
> 
> 
>> +			fdret = FLOW_DISSECT_RET_OUT_BAD;
>> +			break;
>> +		}
>> +
>> +		nhoff += ntohs(hdr->message_length);
>> +		fdret = FLOW_DISSECT_RET_OUT_GOOD;
>> +		break;
>> +	}
>> +
>>   	default:
>>   		fdret = FLOW_DISSECT_RET_OUT_BAD;
>>   		break;
>> -- 
>> 2.17.1
>>