Hi Paul, On 2024-06-17 14:41:52 +0100, Paul Barker wrote: <snip> > >>> + entry = priv->cur_tx % priv->num_tx_ring; > >>> + priv->tx_skb[entry] = skb; > >>> + desc = &priv->tx_ring[entry]; > >>> + desc->dptr = cpu_to_le32(dma_addr); > >>> + desc->info_ds = cpu_to_le16(skb->len); > >> > >> Should we check against the maximum TX frame size supported by the > >> hardware here? > >> > >> Whatever we do here, we should also do in the ravb driver as that makes > >> a similar cpu_to_le16() call to fill the DS field with no check that the > >> HW actually supports transmitting a frame of the given size. > > > > Compared to RAVB the RTSN driver do not support splitting a packet over > > multiple descriptors, so the max frame size adhering to the MTU will > > always fit using a single descriptor. > > > > My concern here is with pathological or malicious packets. A malicious Tx packet due to local VLAN setup? There must be easier ways to crash a machine if you got the permissions to configure interface VLAN settings :-) But I agree the user shall be protected from misconfiguration. > For example, > you can use stacked VLANS (QinQ, QinQinQ, etc) to expand the size of the > TX frame for a given MTU since the bytes used by the extra VLAN tags are > not counted as payload bytes. This is interesting, I only played with single and double tagging never QinQinQ.. . For some reason I assumed that after double tagging space where going to be consumed from the payload. But indeed setting up 4 levels of VLAN tags shows the payload can be force to stay the same and the skb->len do indeed grow. $ ip link add link end0 name end0.2 type vlan id 2 $ ifconfig end0.2 10.0.2.10 netmask 255.255.255.0 up $ ip link add link end0.2 name end0.3 type vlan id 3 $ ifconfig end0.3 10.0.3.10 netmask 255.255.255.0 up $ ip link add link end0.3 name end0.4 type vlan id 4 $ ifconfig end0.4 10.0.4.10 netmask 255.255.255.0 up $ ip link add link end0.4 name end0.5 type vlan id 5 $ ifconfig end0.5 10.0.5.10 netmask 255.255.255.0 up $ ping -s 1500 10.0.5.1 # Give an skb->len of 1530 $ ping -s 1500 10.0.4.1 # Give an skb->len of 1526 $ ping -s 1500 10.0.3.1 # Give an skb->len of 1522 $ ping -s 1500 10.0.2.1 # Give an skb->len of 1518 $ ping -s 1500 10.0.1.1 # Give an skb->len of 1514 (no tags) The above was produced using RAVB and not RTSN and similar VLANS on a 2nd device was setup to allow for ICMP traffic and replies. > > At least with the RZ/G2L family, no verification has been performed on > sending packets larger than 1526 bytes to my knowledge. Even using only > one TX descriptor, I was able to completely lock up the GbEth IP by > pushing a 2kB frame into the TX queue. So I do think it is worth adding > some checks here. I will add a check for this to RTSN. -- Kind Regards, Niklas Söderlund
