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.

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

Jump to bottom

Fix: mac address overflow on windows #3983

Merged
merged 2 commits into from
Nov 14, 2023
Merged

Fix: mac address overflow on windows #3983

merged 2 commits into from
Nov 14, 2023

Conversation

ZgblKylin
Copy link
Contributor

@ZgblKylin ZgblKylin commented Nov 2, 2023 •
edited by JesusPoderoso
Loading

Description

This pull request fixed #3982: Mac address overflow in IPFinder::getAllMACAddress() on Windows.

IP_ADAPTER_ADDRESSES::PhysicalAddressLength will get max up to 8 bytes, but we have only 6 bytes for mac address.

In some case, GetAdaptersAddresses() will report address with PhysicalAddressLength=8, e.g. "Microsoft Teredo Tunneling Adapter", which will cause crash at DomainParticipantFactory::create_participant(). All 8 bytes of IP_ADAPTER_ADDRESSES::PhysicalAddress here are 0 in this situation, so use minimal length 6 does not affect the result.

This bug fix could be backported to older versions.

This fix is valid for all versions since v2.6.6 up to master, older versions not checked yet. It should be merged to all branches having IPFinder::getAllMACAddress().

Contributor Checklist

  • Commit messages follow the project guidelines.
  • The code follows the style guidelines of this project.
  • Tests that thoroughly check the new feature have been added/Regression tests checking the bug and its fix have been added; the added tests pass locally
  • Any new/modified methods have been properly documented using Doxygen.
  • Changes are ABI compatible.
  • Changes are API compatible.
  • New feature has been added to the versions.md file (if applicable).
    • No new feature added.
  • New feature has been documented/Current behavior is correctly described in the documentation.
    • No new feature added.
  • Applicable backports have been included in the description.

Reviewer Checklist

  • The PR has a milestone assigned.
  • Check contributor checklist is correct.
  • Check CI results: changes do not issue any warning.
  • Check CI results: failing tests are unrelated with the changes.

@JLBuenoLopez JLBuenoLopez added this to the v2.12.1 milestone Nov 2, 2023
@ZgblKylin ZgblKylin force-pushed the patch-1 branch 3 times, most recently from 788047d to 8916bca Compare November 2, 2023 08:24
@ZgblKylin
Fix: mac address overflow on windows
41bcf76 
PIP_ADAPTER_ADDRESSES->PhysicalAddressLength will get max up to 8 bytes, but we have only 6 bytes for mac address.
In some case, GetAdaptersAddresses() will report address with PhysicalAddressLength=8, e.g. "Microsoft Teredo Tunneling Adapter", which will cause crash at DomainParticipantFactory::create_participant()

Signed-off-by: [email protected] <[email protected]>
Signed-off-by: Fei Chong <[email protected]>
@ZgblKylin
Copy link
Contributor Author

Sorry for force-pushes, DCO and gpg sign are signed correctly now, no more push needed.

@ZgblKylin ZgblKylin mentioned this pull request Nov 2, 2023
Merged
7 tasks
@ZgblKylin
Copy link
Contributor Author

ZgblKylin commented Nov 2, 2023 •
edited
Loading

Update description, adding line:

All 8 bytes of IP_ADAPTER_ADDRESSES::PhysicalAddress are 0 in this situation, so use minimal length 6 does not affect the result.

MiguelCompany
MiguelCompany previously approved these changes Nov 13, 2023
View reviewed changes
@MiguelCompany
Copy link
Member

@Mergifyio backport 2.11.x 2.10.x 2.6.x

@mergify Mergify
Copy link
Contributor

mergify bot commented Nov 13, 2023 •
edited
Loading

backport 2.11.x 2.10.x 2.6.x

✅ Backports have been created

@MiguelCompany MiguelCompany added the ci-pending PR which CI is running label Nov 13, 2023
@MiguelCompany
Copy link
Member

@richiprosima Please test this

@MiguelCompany
Copy link
Member

@richiprosima Please test windows

@EduPonz EduPonz merged commit 2601e95 into eProsima:master Nov 14, 2023
7 of 11 checks passed
mergify bot pushed a commit that referenced this pull request Nov 14, 2023
@ZgblKylin @mergify
Fix: mac address overflow on windows (#3983)
55a29d9 
* Fix: mac address overflow on windows

PIP_ADAPTER_ADDRESSES->PhysicalAddressLength will get max up to 8 bytes, but we have only 6 bytes for mac address.
In some case, GetAdaptersAddresses() will report address with PhysicalAddressLength=8, e.g. "Microsoft Teredo Tunneling Adapter", which will cause crash at DomainParticipantFactory::create_participant()

Signed-off-by: [email protected] <[email protected]>
Signed-off-by: Fei Chong <[email protected]>

* Fix build error due to windows `min` macro

Signed-off-by: Miguel Company <[email protected]>

---------

Signed-off-by: [email protected] <[email protected]>
Signed-off-by: Fei Chong <[email protected]>
Signed-off-by: Miguel Company <[email protected]>
Co-authored-by: Miguel Company <[email protected]>
(cherry picked from commit 2601e95)
This was referenced Nov 14, 2023
Merged
Merged
Merged
EduPonz pushed a commit that referenced this pull request Dec 2, 2023
@ZgblKylin @EduPonz
Fix: mac address overflow on windows (#3983)
6112158 
* Fix: mac address overflow on windows

PIP_ADAPTER_ADDRESSES->PhysicalAddressLength will get max up to 8 bytes, but we have only 6 bytes for mac address.
In some case, GetAdaptersAddresses() will report address with PhysicalAddressLength=8, e.g. "Microsoft Teredo Tunneling Adapter", which will cause crash at DomainParticipantFactory::create_participant()

Signed-off-by: [email protected] <[email protected]>
Signed-off-by: Fei Chong <[email protected]>

* Fix build error due to windows `min` macro

Signed-off-by: Miguel Company <[email protected]>

---------

Signed-off-by: [email protected] <[email protected]>
Signed-off-by: Fei Chong <[email protected]>
Signed-off-by: Miguel Company <[email protected]>
Co-authored-by: Miguel Company <[email protected]>
(cherry picked from commit 2601e95)
EduPonz pushed a commit that referenced this pull request Dec 3, 2023
@mergify @ZgblKylin
Fix: mac address overflow on windows (#3983) (#4020)
d73f0d4 
* Fix: mac address overflow on windows

PIP_ADAPTER_ADDRESSES->PhysicalAddressLength will get max up to 8 bytes, but we have only 6 bytes for mac address.
In some case, GetAdaptersAddresses() will report address with PhysicalAddressLength=8, e.g. "Microsoft Teredo Tunneling Adapter", which will cause crash at DomainParticipantFactory::create_participant()

Signed-off-by: [email protected] <[email protected]>
Signed-off-by: Fei Chong <[email protected]>

* Fix build error due to windows `min` macro

Signed-off-by: Miguel Company <[email protected]>

---------

Signed-off-by: [email protected] <[email protected]>
Signed-off-by: Fei Chong <[email protected]>
Signed-off-by: Miguel Company <[email protected]>
Co-authored-by: Miguel Company <[email protected]>
(cherry picked from commit 2601e95)

Co-authored-by: Fei Chong <[email protected]>
cferreiragonz pushed a commit that referenced this pull request Dec 5, 2023
@ZgblKylin @cferreiragonz
Fix: mac address overflow on windows (#3983)
a15e271 
* Fix: mac address overflow on windows

PIP_ADAPTER_ADDRESSES->PhysicalAddressLength will get max up to 8 bytes, but we have only 6 bytes for mac address.
In some case, GetAdaptersAddresses() will report address with PhysicalAddressLength=8, e.g. "Microsoft Teredo Tunneling Adapter", which will cause crash at DomainParticipantFactory::create_participant()

Signed-off-by: [email protected] <[email protected]>
Signed-off-by: Fei Chong <[email protected]>

* Fix build error due to windows `min` macro

Signed-off-by: Miguel Company <[email protected]>

---------

Signed-off-by: [email protected] <[email protected]>
Signed-off-by: Fei Chong <[email protected]>
Signed-off-by: Miguel Company <[email protected]>
Co-authored-by: Miguel Company <[email protected]>
(cherry picked from commit 2601e95)
EduPonz pushed a commit that referenced this pull request Dec 11, 2023
@mergify @ZgblKylin
Fix: mac address overflow on windows (#3983) (#4019)
b70bdfc 
* Fix: mac address overflow on windows

PIP_ADAPTER_ADDRESSES->PhysicalAddressLength will get max up to 8 bytes, but we have only 6 bytes for mac address.
In some case, GetAdaptersAddresses() will report address with PhysicalAddressLength=8, e.g. "Microsoft Teredo Tunneling Adapter", which will cause crash at DomainParticipantFactory::create_participant()

Signed-off-by: [email protected] <[email protected]>
Signed-off-by: Fei Chong <[email protected]>

* Fix build error due to windows `min` macro

Signed-off-by: Miguel Company <[email protected]>

---------

Signed-off-by: [email protected] <[email protected]>
Signed-off-by: Fei Chong <[email protected]>
Signed-off-by: Miguel Company <[email protected]>
Co-authored-by: Miguel Company <[email protected]>
(cherry picked from commit 2601e95)

Co-authored-by: Fei Chong <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@EduPonz EduPonz EduPonz approved these changes

@MiguelCompany MiguelCompany MiguelCompany left review comments

Assignees
No one assigned
Labels
ci-pending PR which CI is running
Projects
None yet
Milestone
v2.12.1
Development

Successfully merging this pull request may close these issues.

Mac address overflow in IPFinder::getAllMACAddress() on Windows
4 participants
@ZgblKylin @MiguelCompany @EduPonz @JLBuenoLopez