Skip to content
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.

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

Crash due to assertion failure #533

Closed
black-binary opened this issue Sep 25, 2021 · 2 comments · Fixed by #534
Closed

Crash due to assertion failure #533

black-binary opened this issue Sep 25, 2021 · 2 comments · Fixed by #534

Comments

@black-binary
Copy link

My code crashed after inputting some packets.

Here is a part of the stack backtrace:

Abort message: 'assertion failed: addr.is_unicast()'

(...)
core::panicking::panic::h945a80aaa5884f5b+48
smoltcp::iface::route::Routes::lookup::hc3ac269274d45dbb+1240
smoltcp::iface::interface::InterfaceInner::process_ip::hc48da65fc6377e67+1844
(my rx token consume function)
(interface::poll function)
(...)

lookup was called from here.

smoltcp/src/iface/interface.rs

Lines 1235 to 1249 in dedfbe1

if !self.has_ip_addr(ipv4_repr.dst_addr)
&& !self.has_multicast_group(ipv4_repr.dst_addr)
&& !self.is_broadcast_v4(ipv4_repr.dst_addr)
{
// Ignore IP packets not directed at us, or broadcast, or any of the multicast groups.
// If AnyIP is enabled, also check if the packet is routed locally.
if !self.any_ip
|| self
.routes
.lookup(&IpAddress::Ipv4(ipv4_repr.dst_addr), cx.now)
.map_or(true, |router_addr| !self.has_ip_addr(router_addr))
{
return Ok(None);
}
}

The assertion failed at the beginning of the lookup function and caused panic.

smoltcp/src/iface/route.rs

Lines 129 to 133 in dedfbe1

pub(crate) fn lookup(&self, addr: &IpAddress, timestamp: Instant) -> Option<IpAddress> {
assert!(addr.is_unicast());
let cidr = match addr {
#[cfg(feature = "proto-ipv4")]

@black-binary
Copy link
Author

Maybe we should fix this by adding more conditions.

smoltcp/src/iface/interface.rs

Lines 1235 to 1237 in dedfbe1

if !self.has_ip_addr(ipv4_repr.dst_addr)
&& !self.has_multicast_group(ipv4_repr.dst_addr)
&& !self.is_broadcast_v4(ipv4_repr.dst_addr)

->

 if !self.has_ip_addr(ipv4_repr.dst_addr) 
     && !self.has_multicast_group(ipv4_repr.dst_addr) 
     && !self.is_broadcast_v4(ipv4_repr.dst_addr) 
     && ipv4_repr.dst_addr.is_unicast()

bors bot added a commit that referenced this issue Sep 26, 2021
534: Fix assert with any_ip + broadcast dst_addr. r=Dirbaio a=Dirbaio

Fixes #533

Co-authored-by: Dario Nieuwenhuis <dirbaio@dirbaio.net>
@Dirbaio
Copy link
Member

Dirbaio commented Sep 26, 2021

Nice catch! The fix is adding the condition in the inner if, since we do want to ignore these packets (multicast dst_addr that we're not subscribed to). Fix in #534

bors bot added a commit that referenced this issue Sep 26, 2021
534: Fix assert with any_ip + broadcast dst_addr. r=Dirbaio a=Dirbaio

Fixes #533

Co-authored-by: Dario Nieuwenhuis <dirbaio@dirbaio.net>
@bors bors bot closed this as completed in 21e0a30 Sep 26, 2021
@bors bors bot closed this as completed in #534 Sep 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants