Skip to content
New issue

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

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

Already on GitHub? # to your account

Fix nil pointer dereference when ClusterGroup/Group is used #6077

Merged
merged 1 commit into from
Mar 8, 2024

Conversation

tnqn
Copy link
Member

@tnqn tnqn commented Mar 7, 2024

When an AppliedToGroup or AddressGroup is derived from a ClusterGroup or Group, we used the existence of the source Group in the internal group storage as the indicator of the type of AppliedToGroup or AddressGroup. After the source Group is deleted, the AppliedToGroup or AddressGroup was considered as a Group with its own selector mistakenly. Accessing its selector would panic due to nil pointer dereference.

This patch makes the type of AppliedToGroup and AddressGroup more explicit by adding a field "SourceGroup" to indicate it. If the source Group can't be found in the storage, we just return nil to indicate the Group selects nothing at the moment.

Fixes #6075

@tnqn tnqn added area/component/controller Issues or PRs related to the controller component area/network-policy Issues or PRs related to network policies. area/network-policy/controller Issues or PRs related to the network policy controller action/backport Indicates a PR that requires backports. action/release-note Indicates a PR that should be included in release notes. labels Mar 7, 2024
When an AppliedToGroup or AddressGroup is derived from a ClusterGroup or
Group, we used whether the existence of the source Group in the internal
group storage as the indicator of the type of AppliedToGroup or
AddressGroup. After the source Group is deleted, the AppliedToGroup or
AddressGroup was considered as a Group with its own selector mistakenly.
Accessing its selector would panic due to nil pointer dereference.

This patch makes the type of AppliedToGroup and AddressGroup more
explicit by adding a field "SourceGroup" to indicate it. If the source
Group can't be found in the storage, we just return nil to indicate the
Group selects nothing at the moment.

Signed-off-by: Quan Tian <qtian@vmware.com>
Copy link
Contributor

@antoninbas antoninbas left a comment

Choose a reason for hiding this comment

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

LGTM
One nit in the PR description:
s/we used whether the existence of the source Group/we used the existence of the source Group

@tnqn
Copy link
Member Author

tnqn commented Mar 8, 2024

LGTM One nit in the PR description: s/we used whether the existence of the source Group/we used the existence of the source Group

Thanks, updated PR description, will edit commit message when merging it.

/test-all

@tnqn tnqn merged commit 17ff95a into antrea-io:main Mar 8, 2024
51 of 55 checks passed
@tnqn tnqn deleted the fix-panic-in-controller branch March 8, 2024 05:54
tnqn added a commit to tnqn/antrea that referenced this pull request Mar 8, 2024
…o#6077)

When an AppliedToGroup or AddressGroup is derived from a ClusterGroup or
Group, we used the existence of the source Group in the internal group
storage as the indicator of the type of AppliedToGroup or AddressGroup.
After the source Group is deleted, the AppliedToGroup or AddressGroup
was considered as a Group with its own selector mistakenly. Accessing
its selector would panic due to nil pointer dereference.

This patch makes the type of AppliedToGroup and AddressGroup more
explicit by adding a field "SourceGroup" to indicate it. If the source
Group can't be found in the storage, we just return nil to indicate the
Group selects nothing at the moment.

Signed-off-by: Quan Tian <qtian@vmware.com>
tnqn added a commit to tnqn/antrea that referenced this pull request Mar 8, 2024
…o#6077)

When an AppliedToGroup or AddressGroup is derived from a ClusterGroup or
Group, we used the existence of the source Group in the internal group
storage as the indicator of the type of AppliedToGroup or AddressGroup.
After the source Group is deleted, the AppliedToGroup or AddressGroup
was considered as a Group with its own selector mistakenly. Accessing
its selector would panic due to nil pointer dereference.

This patch makes the type of AppliedToGroup and AddressGroup more
explicit by adding a field "SourceGroup" to indicate it. If the source
Group can't be found in the storage, we just return nil to indicate the
Group selects nothing at the moment.

Signed-off-by: Quan Tian <qtian@vmware.com>
tnqn added a commit to tnqn/antrea that referenced this pull request Mar 8, 2024
…o#6077)

When an AppliedToGroup or AddressGroup is derived from a ClusterGroup or
Group, we used the existence of the source Group in the internal group
storage as the indicator of the type of AppliedToGroup or AddressGroup.
After the source Group is deleted, the AppliedToGroup or AddressGroup
was considered as a Group with its own selector mistakenly. Accessing
its selector would panic due to nil pointer dereference.

This patch makes the type of AppliedToGroup and AddressGroup more
explicit by adding a field "SourceGroup" to indicate it. If the source
Group can't be found in the storage, we just return nil to indicate the
Group selects nothing at the moment.

Signed-off-by: Quan Tian <qtian@vmware.com>
@tnqn tnqn added the kind/bug Categorizes issue or PR as related to a bug. label Jul 16, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
action/backport Indicates a PR that requires backports. action/release-note Indicates a PR that should be included in release notes. area/component/controller Issues or PRs related to the controller component area/network-policy/controller Issues or PRs related to the network policy controller area/network-policy Issues or PRs related to network policies. kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

antrea-controller panic when running NetworkPolicy e2e
2 participants