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

fix for malformed IPv6 Network Solicitation packet #10160

Merged
merged 1 commit into from
Oct 6, 2021

Conversation

billwatersiii
Copy link
Contributor

Problem

This fixes the issue reported here - #9791

The current version of lwip has a bug that creates a malformed Network Solicitation packet that can cause certain cable-modem/router models to crash (ex. https://www.motorola.com/us/mg7550/p). It takes a power cycle to recover from the crash.

Change overview

A code update to fix the issue.

Testing

This was manually tested with the examples/lock-app/p6 application. The problem occurs on the "Enable WiFi Network" step during the provisioning before Test Event tests are run.

zcl NetworkCommissioning EnableNetwork 1234 0 0 networkID=str:<WIFI SSID> breadcrumb=0 timeoutMs=1000

The problem no longer occurs after the code update in this pull request.

@CLAassistant
Copy link

CLAassistant commented Oct 1, 2021

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@andy31415 andy31415 left a comment

Choose a reason for hiding this comment

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

Approving, however I believe that LWIP used is generally part of various SDKs and not this particular file.

@cecille - is this true? Do we have some document about how LWIP is pulled in and how to consider various changes to it?

@andy31415
Copy link
Contributor

I assume this fixes at least p6. Others may not be fixed/have the required change.

Was this change upstreamed to the official LWIP repository?

@andy31415
Copy link
Contributor

@pan-apple - I see you may have imported this repo to start with. How are changes to the repo expected? Why was git submodules not used?

@billwatersiii
Copy link
Contributor Author

Was this change upstreamed to the official LWIP repository?

The latest version of LWIP does not have this issue.

Copy link
Contributor

@cecille cecille left a comment

Choose a reason for hiding this comment

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

I think this was fixed on mainline in https://git.savannah.nongnu.org/cgit/lwip.git/commit/?id=bc08c1d2b79b4763fc0f8f0bf0ed58e0c2899b3a - may want to just pull that patch rather than re-patch here.

@billwatersiii
Copy link
Contributor Author

I think this was fixed on mainline in https://git.savannah.nongnu.org/cgit/lwip.git/commit/?id=bc08c1d2b79b4763fc0f8f0bf0ed58e0c2899b3a - may want to just pull that patch rather than re-patch here.

@cecille, the version of lwip that this patch was applied to is different from the version in the Matter repo. It seemed safer to only apply the changes from latest LWIP tag that fix the issue.

@cecille
Copy link
Contributor

cecille commented Oct 4, 2021

Ok, yeah - fair if it doesn't apply cleanly. However, that patch applies a similar fix to a few other places in the code. Ex. you can see the same pattern in the send_na function - does it need to be changed there as well?

@billwatersiii
Copy link
Contributor Author

Ok, yeah - fair if it doesn't apply cleanly. However, that patch applies a similar fix to a few other places in the code. Ex. you can see the same pattern in the send_na function - does it need to be changed there as well?

@cecille, I have been directed to only fix "point issues", and this "zero length Network Solicitation packet" is the only issue that I am aware of. For broader fixes/updates, I would recommend that Matter move to a more recent version of lwip.

@cecille
Copy link
Contributor

cecille commented Oct 5, 2021

Up until this new platform, I wasn't actually aware of anyone using this version of lwip - network stack stuff for most platforms is supplied by the SDK. I actually thought this was dead legacy code. I'd be fine with updating, personally. There's some excellent ipv6 bugs in older versions of lwip, and this one looks relatively old.

@billwatersiii
Copy link
Contributor Author

Up until this new platform, I wasn't actually aware of anyone using this version of lwip - network stack stuff for most platforms is supplied by the SDK. I actually thought this was dead legacy code. I'd be fine with updating, personally. There's some excellent ipv6 bugs in older versions of lwip, and this one looks relatively old.

@cecille,

Here's what our plan is...

  1. Let's fix this bug now, since we know about it and we know of instances where we encounter it.

  2. Infineon will update our demo app to use the LWiP from our standard SDK prior to 1.0.

  3. After the Infineon update, we can delete the Matter LWiP, given it is dead code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants