Skip to content

Commit 04b2f2e

Browse files
committed
net: icmp: Verify the address family before calling the callback
Check the address family of the packet before passing it to a ICMP handler, to avoid scenarios where ICMPv4 packet is paseed to a ICMPv6 handler and vice versa. Signed-off-by: Robert Lubos <[email protected]> (cherry picked from commit 587d9e6)
1 parent 9bacf34 commit 04b2f2e

File tree

17 files changed

+46
-30
lines changed

17 files changed

+46
-30
lines changed

include/zephyr/net/icmp.h

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,9 @@ struct net_icmp_ctx {
9393
/** Opaque user supplied data */
9494
void *user_data;
9595

96+
/** Address family the handler is registered for */
97+
uint8_t family;
98+
9699
/** ICMP type of the response we are waiting */
97100
uint8_t type;
98101

@@ -157,12 +160,13 @@ struct net_icmp_ping_params {
157160
* system.
158161
*
159162
* @param ctx ICMP context used in this request.
163+
* @param family Address family the context is using.
160164
* @param type Type of ICMP message we are handling.
161165
* @param code Code of ICMP message we are handling.
162166
* @param handler Callback function that is called when a response is received.
163167
*/
164-
int net_icmp_init_ctx(struct net_icmp_ctx *ctx, uint8_t type, uint8_t code,
165-
net_icmp_handler_t handler);
168+
int net_icmp_init_ctx(struct net_icmp_ctx *ctx, uint8_t family, uint8_t type,
169+
uint8_t code, net_icmp_handler_t handler);
166170

167171
/**
168172
* @brief Cleanup the ICMP context structure. This will unregister the ICMP handler

subsys/net/ip/icmp.c

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,16 +50,22 @@ static sys_slist_t offload_handlers = SYS_SLIST_STATIC_INIT(&offload_handlers);
5050

5151
#define PKT_WAIT_TIME K_SECONDS(1)
5252

53-
int net_icmp_init_ctx(struct net_icmp_ctx *ctx, uint8_t type, uint8_t code,
54-
net_icmp_handler_t handler)
53+
int net_icmp_init_ctx(struct net_icmp_ctx *ctx, uint8_t family, uint8_t type,
54+
uint8_t code, net_icmp_handler_t handler)
5555
{
5656
if (ctx == NULL || handler == NULL) {
5757
return -EINVAL;
5858
}
5959

60+
if (family != AF_INET && family != AF_INET6) {
61+
NET_ERR("Wrong address family");
62+
return -EINVAL;
63+
}
64+
6065
memset(ctx, 0, sizeof(struct net_icmp_ctx));
6166

6267
ctx->handler = handler;
68+
ctx->family = family;
6369
ctx->type = type;
6470
ctx->code = code;
6571

@@ -511,6 +517,10 @@ static int icmp_call_handlers(struct net_pkt *pkt,
511517
k_mutex_lock(&lock, K_FOREVER);
512518

513519
SYS_SLIST_FOR_EACH_CONTAINER(&handlers, ctx, node) {
520+
if (ip_hdr->family != ctx->family) {
521+
continue;
522+
}
523+
514524
if (ctx->type == icmp_hdr->type &&
515525
(ctx->code == icmp_hdr->code || ctx->code == 0U)) {
516526
/* Do not use a handler that is expecting data from different

subsys/net/ip/icmpv4.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -762,14 +762,15 @@ void net_icmpv4_init(void)
762762
static struct net_icmp_ctx ctx;
763763
int ret;
764764

765-
ret = net_icmp_init_ctx(&ctx, NET_ICMPV4_ECHO_REQUEST, 0, icmpv4_handle_echo_request);
765+
ret = net_icmp_init_ctx(&ctx, AF_INET, NET_ICMPV4_ECHO_REQUEST, 0,
766+
icmpv4_handle_echo_request);
766767
if (ret < 0) {
767768
NET_ERR("Cannot register %s handler (%d)", STRINGIFY(NET_ICMPV4_ECHO_REQUEST),
768769
ret);
769770
}
770771

771772
#if defined(CONFIG_NET_IPV4_PMTU)
772-
ret = net_icmp_init_ctx(&dst_unreach_ctx, NET_ICMPV4_DST_UNREACH, 0,
773+
ret = net_icmp_init_ctx(&dst_unreach_ctx, AF_INET, NET_ICMPV4_DST_UNREACH, 0,
773774
icmpv4_handle_dst_unreach);
774775
if (ret < 0) {
775776
NET_ERR("Cannot register %s handler (%d)", STRINGIFY(NET_ICMPV4_DST_UNREACH),

subsys/net/ip/icmpv6.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -384,7 +384,8 @@ void net_icmpv6_init(void)
384384
static struct net_icmp_ctx ctx;
385385
int ret;
386386

387-
ret = net_icmp_init_ctx(&ctx, NET_ICMPV6_ECHO_REQUEST, 0, icmpv6_handle_echo_request);
387+
ret = net_icmp_init_ctx(&ctx, AF_INET6, NET_ICMPV6_ECHO_REQUEST, 0,
388+
icmpv6_handle_echo_request);
388389
if (ret < 0) {
389390
NET_ERR("Cannot register %s handler (%d)", STRINGIFY(NET_ICMPV6_ECHO_REQUEST),
390391
ret);

subsys/net/ip/ipv6_mld.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -461,7 +461,7 @@ void net_ipv6_mld_init(void)
461461
static struct net_icmp_ctx ctx;
462462
int ret;
463463

464-
ret = net_icmp_init_ctx(&ctx, NET_ICMPV6_MLD_QUERY, 0, handle_mld_query);
464+
ret = net_icmp_init_ctx(&ctx, AF_INET6, NET_ICMPV6_MLD_QUERY, 0, handle_mld_query);
465465
if (ret < 0) {
466466
NET_ERR("Cannot register %s handler (%d)", STRINGIFY(NET_ICMPV6_MLD_QUERY),
467467
ret);

subsys/net/ip/ipv6_nbr.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2865,13 +2865,13 @@ void net_ipv6_nbr_init(void)
28652865
int ret;
28662866

28672867
#if defined(CONFIG_NET_IPV6_NBR_CACHE)
2868-
ret = net_icmp_init_ctx(&ns_ctx, NET_ICMPV6_NS, 0, handle_ns_input);
2868+
ret = net_icmp_init_ctx(&ns_ctx, AF_INET6, NET_ICMPV6_NS, 0, handle_ns_input);
28692869
if (ret < 0) {
28702870
NET_ERR("Cannot register %s handler (%d)", STRINGIFY(NET_ICMPV6_NS),
28712871
ret);
28722872
}
28732873

2874-
ret = net_icmp_init_ctx(&na_ctx, NET_ICMPV6_NA, 0, handle_na_input);
2874+
ret = net_icmp_init_ctx(&na_ctx, AF_INET6, NET_ICMPV6_NA, 0, handle_na_input);
28752875
if (ret < 0) {
28762876
NET_ERR("Cannot register %s handler (%d)", STRINGIFY(NET_ICMPV6_NA),
28772877
ret);
@@ -2880,7 +2880,7 @@ void net_ipv6_nbr_init(void)
28802880
k_work_init_delayable(&ipv6_ns_reply_timer, ipv6_ns_reply_timeout);
28812881
#endif
28822882
#if defined(CONFIG_NET_IPV6_ND)
2883-
ret = net_icmp_init_ctx(&ra_ctx, NET_ICMPV6_RA, 0, handle_ra_input);
2883+
ret = net_icmp_init_ctx(&ra_ctx, AF_INET6, NET_ICMPV6_RA, 0, handle_ra_input);
28842884
if (ret < 0) {
28852885
NET_ERR("Cannot register %s handler (%d)", STRINGIFY(NET_ICMPV6_RA),
28862886
ret);
@@ -2891,7 +2891,7 @@ void net_ipv6_nbr_init(void)
28912891
#endif
28922892

28932893
#if defined(CONFIG_NET_IPV6_PMTU)
2894-
ret = net_icmp_init_ctx(&ptb_ctx, NET_ICMPV6_PACKET_TOO_BIG, 0, handle_ptb_input);
2894+
ret = net_icmp_init_ctx(&ptb_ctx, AF_INET6, NET_ICMPV6_PACKET_TOO_BIG, 0, handle_ptb_input);
28952895
if (ret < 0) {
28962896
NET_ERR("Cannot register %s handler (%d)", STRINGIFY(NET_ICMPV6_PACKET_TOO_BIG),
28972897
ret);

subsys/net/lib/dhcpv4/dhcpv4_server.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -879,7 +879,7 @@ static int echo_reply_handler(struct net_icmp_ctx *icmp_ctx,
879879

880880
static int dhcpv4_server_probing_init(struct dhcpv4_server_ctx *ctx)
881881
{
882-
return net_icmp_init_ctx(&ctx->probe_ctx.icmp_ctx,
882+
return net_icmp_init_ctx(&ctx->probe_ctx.icmp_ctx, AF_INET,
883883
NET_ICMPV4_ECHO_REPLY, 0,
884884
echo_reply_handler);
885885
}

subsys/net/lib/shell/ping.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -456,7 +456,7 @@ static int cmd_net_ping(const struct shell *sh, size_t argc, char *argv[])
456456
net_addr_pton(AF_INET6, host, &ping_ctx.addr6.sin6_addr) == 0) {
457457
ping_ctx.addr6.sin6_family = AF_INET6;
458458

459-
ret = net_icmp_init_ctx(&ping_ctx.icmp, NET_ICMPV6_ECHO_REPLY, 0,
459+
ret = net_icmp_init_ctx(&ping_ctx.icmp, AF_INET6, NET_ICMPV6_ECHO_REPLY, 0,
460460
handle_ipv6_echo_reply);
461461
if (ret < 0) {
462462
PR_WARNING("Cannot initialize ICMP context for %s\n", "IPv6");
@@ -466,7 +466,7 @@ static int cmd_net_ping(const struct shell *sh, size_t argc, char *argv[])
466466
net_addr_pton(AF_INET, host, &ping_ctx.addr4.sin_addr) == 0) {
467467
ping_ctx.addr4.sin_family = AF_INET;
468468

469-
ret = net_icmp_init_ctx(&ping_ctx.icmp, NET_ICMPV4_ECHO_REPLY, 0,
469+
ret = net_icmp_init_ctx(&ping_ctx.icmp, AF_INET, NET_ICMPV4_ECHO_REPLY, 0,
470470
handle_ipv4_echo_reply);
471471
if (ret < 0) {
472472
PR_WARNING("Cannot initialize ICMP context for %s\n", "IPv4");

subsys/net/lib/zperf/zperf_shell.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -679,7 +679,7 @@ static void send_ping(const struct shell *sh,
679679
struct net_icmp_ctx ctx;
680680
int ret;
681681

682-
ret = net_icmp_init_ctx(&ctx, NET_ICMPV6_ECHO_REPLY, 0, ping_handler);
682+
ret = net_icmp_init_ctx(&ctx, AF_INET6, NET_ICMPV6_ECHO_REPLY, 0, ping_handler);
683683
if (ret < 0) {
684684
shell_fprintf(sh, SHELL_WARNING, "Cannot send ping (%d)\n", ret);
685685
return;

tests/boards/espressif/ethernet/src/main.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ ZTEST(ethernet, test_icmp_check)
9797
gw_addr_4 = net_if_ipv4_get_gw(iface);
9898
zassert_not_equal(gw_addr_4.s_addr, 0, "Gateway address is not set");
9999

100-
ret = net_icmp_init_ctx(&ctx, NET_ICMPV4_ECHO_REPLY, 0, icmp_event);
100+
ret = net_icmp_init_ctx(&ctx, AF_INET, NET_ICMPV4_ECHO_REPLY, 0, icmp_event);
101101
zassert_equal(ret, 0, "Cannot init ICMP (%d)", ret);
102102

103103
dst4.sin_family = AF_INET;

0 commit comments

Comments
 (0)