[systemd-devel] [PATCH] dhcp-network: ignore IP packets with More Fragments (MF) flag set

[Tom Gundersen]

On Mon, Jul 7, 2014 at 2:45 PM, Michal Sekletar <msekleta at redhat.com> wrote:
> We already ignore IP fragments, because we expect that Fragment
> offset (FO) field is not set. However first fragment in a fragmented IP
> flow will have all zeroes in FO field. We should ignore such packet as
> well, thus we need to look at MF flag in the IP header. Checking MF flag
> will filter out all except last packet in fragmented flows. Last one
> will be ruled out by next check for value of FO.
> ---
>  src/libsystemd-network/dhcp-network.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/src/libsystemd-network/dhcp-network.c b/src/libsystemd-network/dhcp-network.c
> index f119cae..455d5a8 100644
> --- a/src/libsystemd-network/dhcp-network.c
> +++ b/src/libsystemd-network/dhcp-network.c
> @@ -41,6 +41,10 @@ int dhcp_network_bind_raw_socket(int index, union sockaddr_union *link,
>              BPF_STMT(BPF_LD + BPF_B + BPF_ABS, offsetof(DHCPPacket, ip.protocol)), /* A <- IP protocol */
>              BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, IPPROTO_UDP, 1, 0),                /* IP protocol == UDP ? */
>              BPF_STMT(BPF_RET + BPF_K, 0),                                          /* ignore */
> +            BPF_STMT(BPF_LD + BPF_B + BPF_ABS, offsetof(DHCPPacket, ip.frag_off)), /* A <- Flags */
> +            BPF_STMT(BPF_ALU + BPF_AND + BPF_K, 0x20),                             /* A <- A & 0x20 */

Maybe express better what we are checking here? Something like /* A <-
A & 0x20 (More Fragments bit) */

> +            BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, 0, 1, 0),                          /* A == 0 ? */
> +            BPF_STMT(BPF_RET + BPF_K, 0),                                          /* ignore */
>              BPF_STMT(BPF_LD + BPF_H + BPF_ABS, offsetof(DHCPPacket, ip.frag_off)), /* A <- Flags + Fragment offset */
>              BPF_STMT(BPF_ALU + BPF_AND + BPF_K, 0x1fff),                           /* A <- A & 0x1fff */

Actually, here we should probably comment /* A <- A & 0x1fff (Fragment
offset) */

>              BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, 0, 1, 0),                          /* A == 0 ? */

Apart from that, it looks good. So please push with updated comments.

Cheers,

Tom