Skip to content

bfd: fix RFC 5880 compliance violations in packet reception #20324

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
SpadeMomo wants to merge 1 commit into
base: master
Choose a base branch
from
Open

bfd: fix RFC 5880 compliance violations in packet reception #20324

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 11 additions & 5 deletions bfdd/bfd.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,12 +192,18 @@ struct bfd_echo_pkt {
SET_FLAG(flags, (CHECK_FLAG(val, 0x3) << 6)); \
}
#define BFD_GETSTATE(flags) (CHECK_FLAG((flags >> 6), 0x3))
#define BFD_SETCBIT(flags, val) \
{ \
if ((val)) \
SET_FLAG(flags, val); \
#define BFD_SETCBIT(flags, val) \
{ \
if ((val)) \
SET_FLAG(flags, BFD_CBIT); \
}
#define BFD_GETCBIT(flags) (CHECK_FLAG(flags, BFD_CBIT))
#define BFD_SETABIT(flags, val) \
{ \
if ((val)) \
SET_FLAG(flags, BFD_ABIT); \
}
#define BFD_GETABIT(flags) (CHECK_FLAG(flags, BFD_ABIT))
#define BFD_ECHO_VERSION 1
#define BFD_ECHO_PKT_LEN sizeof(struct bfd_echo_pkt)

Expand Down Expand Up @@ -891,4 +897,4 @@ struct bfd_perm_vrf {
struct bfd_perm_vrfs_item itm;
char *vrf_name;
};
#endif /* _BFD_H_ */
#endif /* _BFD_H_ */
Copy link

@greptile-apps greptile-apps bot Jan 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing newline at end of file

Suggested change
#endif /* _BFD_H_ */
#endif /* _BFD_H_ */

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix With AI 
This is a comment left during a code review.
Path: bfdd/bfd.h
Line: 900:900

Comment:
Missing newline at end of file

```suggestion
#endif /* _BFD_H_ */

```

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.

44 changes: 36 additions & 8 deletions bfdd/bfd_packet.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -852,6 +852,17 @@ static bool bfd_check_auth(const struct bfd_session *bfd,
return true;
}

static int match_ip_address(const struct sockaddr_any *a, const struct sockaddr_any *b)
{
if (a->sa_sin.sin_family != b->sa_sin.sin_family)
return 0;
if (a->sa_sin.sin_family == AF_INET)
return IPV4_ADDR_SAME(&a->sa_sin.sin_addr, &b->sa_sin.sin_addr);
else if (a->sa_sin.sin_family == AF_INET6)
return IPV6_ADDR_SAME(&a->sa_sin6.sin6_addr, &b->sa_sin6.sin6_addr);
return 0;
}

void bfd_recv_cb(struct event *t)
{
int sd = EVENT_FD(t);
Expand Down Expand Up @@ -954,7 +965,7 @@ void bfd_recv_cb(struct event *t)
return;
}

if ((cp->len < BFD_PKT_LEN) || (cp->len > mlen)) {
if ((cp->len < (BFD_GETABIT(cp->flags) ? BFD_PKT_LEN + 2 : BFD_PKT_LEN)) || (cp->len > mlen)) {
frrtrace(8, frr_bfd, packet_validation_error, 5, is_mhop, &peer, &local, ifindex,
vrfid, cp->len, (uint32_t)mlen);
cp_debug(is_mhop, &peer, &local, ifindex, vrfid, "too small");
Expand Down Expand Up @@ -999,12 +1010,27 @@ void bfd_recv_cb(struct event *t)
}

/* Ensure that existing good sessions are not overridden. */
if (!cp->discrs.remote_discr && bfd->ses_state != PTM_BFD_DOWN &&
bfd->ses_state != PTM_BFD_ADM_DOWN) {
frrtrace(6, frr_bfd, packet_remote_discr_zero, is_mhop, &peer, &local, ifindex,
vrfid, bfd->ses_state);
/* Drop packet if address mismatch */
if (!cp->discrs.remote_discr) {
if (bfd->ses_state == PTM_BFD_UP) {
/* If address mismatch, drop the packet */
if (bfd->local_address.sa_sin.sin_family != AF_UNSPEC &&
(match_ip_address(&bfd->local_address, &local) == 0)) {
frrtrace(6, frr_bfd, packet_remote_discr_zero, is_mhop, &peer,
&local, ifindex, vrfid, bfd->ses_state);
cp_debug(is_mhop, &peer, &local, ifindex, vrfid,
"local address mismatch");
return;
}
}
}

/* Implement RFC 5880 6.8.6 */
/* This is different from last check, because RFC requires checking of packet state too. */
if (!cp->discrs.remote_discr && BFD_GETSTATE(cp->flags) != PTM_BFD_DOWN &&
BFD_GETSTATE(cp->flags) != PTM_BFD_ADM_DOWN) {
cp_debug(is_mhop, &peer, &local, ifindex, vrfid,
"'remote discriminator' is zero, not overridden");
"'remote discriminator' is zero in non-DOWN state");
return;
}

Expand Down Expand Up @@ -1051,8 +1077,7 @@ void bfd_recv_cb(struct event *t)
bfd->discrs.remote_discr, ntohl(cp->discrs.my_discr));
}

bfd->discrs.remote_discr = ntohl(cp->discrs.my_discr);


Copy link

@greptile-apps greptile-apps bot Jan 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trailing whitespace on this line

Suggested change
/* Check authentication. */

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix With AI 
This is a comment left during a code review.
Path: bfdd/bfd_packet.c
Line: 1080:1080

Comment:
Trailing whitespace on this line

```suggestion
	/* Check authentication. */
```

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.

/* Check authentication. */
if (!bfd_check_auth(bfd, cp)) {
/* Extract auth type from packet for tracing */
Expand All @@ -1073,6 +1098,9 @@ void bfd_recv_cb(struct event *t)
return;
}

/* Update remote discriminator after authentication check. */
bfd->discrs.remote_discr = ntohl(cp->discrs.my_discr);

/* Save remote diagnostics before state switch. */
bfd->remote_diag = CHECK_FLAG(cp->diag, BFD_DIAGMASK);

Expand Down
Loading