Hi,
We met some issues during ptp4l test, and we believe these issues shall be
fixed by ptp4l.
1. According the standard, the ptp4l shall not check the suffix for
Delay_req since it is event message.
13.2 General message format requirements
All messages shall have a header, body, and suffix. The suffix may have zero
length.
13.4 Suffix
An application layer message is suffixed by a contiguous sequence of zero or
more entities of data type
TLV. The meaning of the entities is described in Clause 14. The first octet of
TLV entity "n+1" shall
immediately follow the final octet of TLV entity "n". The interpretation of a
TLV should not depend on its
position in the message. Nodes should append no TLV entity to event messages.
whereby from below we know Delay_Req message class is Event, thus no TLVs are
allowed for Delay_Req.
13.3.2.2 messageType (Enumeration4)
The value messageType shall indicate the type of the message as defined in
Table 19.
Table 19 Values of messageType field
Message type Message class Value (hex)
Sync Event 0
Delay_Req Event 1
Pdelay_Req Event 2
Pdelay_Resp Event 3
So here should be the fix for event DELAY_REQ:
static uint8_t *msg_suffix(struct ptp_message *m)
{
switch (msg_type(m)) {
case SYNC:
return NULL;
case DELAY_REQ:
return m->delay_req.suffix; <- should
return NULL
case PDELAY_REQ:
case PDELAY_RESP:
return NULL;
case FOLLOW_UP:
return m->follow_up.suffix;
case DELAY_RESP:
return m->delay_resp.suffix;
case PDELAY_RESP_FOLLOW_UP:
return m->pdelay_resp_fup.suffix;
case ANNOUNCE:
return m->announce.suffix;
case SIGNALING:
return m->signaling.suffix;
case MANAGEMENT:
return m->management.suffix;
}
return NULL;
}
1. The message length is the sum of the PTP pdu length and suffix length,
however ptp4l calculate the suffix length based on the length received from the
transport which may contains other staff like the paddings in Ethernet header
added by our HW.
If these padding added by the HW are less than 4 bytes, there is no complain
from ptp4l. But once they are equal or bigger than 4 bytes, the current logic
in ptp4l treat them as bad messages. So the ptp4l shall calculate the suffix
length based on the message length, not the length from the transport.
suffix_len = suffix_post_recv(m, cnt - pdulen); <- shall be updated to
suffix_len = suffix_post_recv(m, m->header.messageLength - pdulen);
1. The bad message checker does not cover all bad scenarios which may cause
ptp4l crashes since the attribute of message length in PTP message could be
manipulated with huge number, however the cnt length and pdu length are correct.
Eg. the vulnerability test tool could inject wrong value to the attribute of
message length in proper generated message, there's no need to detect this
mal-forming in post-checker, but in pre-check. Otherwise we unnecessary waste
processing time while iterating through "assumed TLVs" in context of
suffix_post_rec
>From security point of view, this fix shall be taken.
int msg_post_recv(struct ptp_message *m, int cnt)
{ .........
if (cnt < pdulen) { <- should be if (cnt <
m->header.messageLength || m->header.messageLength < pdulen)
return -EBADMSG;
}
......
}
Please let me know your idea and suggestion on these issues.
Thanks in advance.
Thanks,
Cindy
_______________________________________________
Linuxptp-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
