Skip to content

PoC code for SWIFT v2 addition to Kata#424

Open
JocelynBerrendonner wants to merge 2 commits intomsft-mainfrom
user/jocelynb/swiftv2
Open

PoC code for SWIFT v2 addition to Kata#424
JocelynBerrendonner wants to merge 2 commits intomsft-mainfrom
user/jocelynb/swiftv2

Conversation

@JocelynBerrendonner
Copy link
Copy Markdown
Member

Merge Checklist
  • Followed patch format from upstream recommendation: https://github.com/kata-containers/community/blob/main/CONTRIBUTING.md#patch-format
  • Included a single commit in a given PR - at least unless there are related commits and each makes sense as a change on its own.
  • Merged using "create a merge commit" rather than "squash and merge" (or similar)
  • genPolicy only: Builds on Windows
  • genPolicy only: Updated sample YAMLs' policy annotations, if applicable
Summary

This PR captures the PoC code for SWIFT v2 support in Kata. The code does two things:

  1. It adds support for vmbus based physical network adapters
  2. It adds support for non-VF (i.e. non SR-IOV) physical network adapters

The code mostly modifies the physical network path to provide the above support. It also uses the same approach as VETH for all the non-VF / non SR-IOV network adapters that are being added to a Kata UVM.

Associated issues
Links to CVEs
Test Methodology

Manual validation of the PoC. No regression testing (this is pure PoC code)

}

if isPhysical {
if s.config.HypervisorConfig.ColdPlugVFIO == config.NoPort {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Shouldn't that still be needed?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It is still needed, but this check was moved into "createPhysicalEndpoint" because the VFIO Disabled config flag only applies to VFIO interfaces, and we can't check the VFIO Disabled config flag before we know the interface is an actual VFIO interface. In the previous code, the flow assumed that "isPhysical" automatically means "is VFIO", which is what I changed here, so this check cannot be done as early in the code anymore.

} else {
// The network namespace would have been deleted at this point
// if it has not been created by virtcontainers.
if !netNsCreated {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Should we add some logging here?

Copy link
Copy Markdown
Member Author

@JocelynBerrendonner JocelynBerrendonner Jan 12, 2026

Choose a reason for hiding this comment

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

I am on the fence for this one. I took the same approach as the existing veth code:

Adding logging here can significantly increase the number of log lines but the usefulness of the log line may be arguable (the netNsCreated flag is an internal flag to prevent detaching a network interface multiple times, as "Detach" can be called multiple times, so, if we add a log line here, we will see a call with "netNsCreated" for both the successful deletion, and the harmless subsequent calls to Detach. This may make the logs confusing).

Signed-off-by: Aurélien Bombo <abombo@microsoft.com>
if err != nil {
return nil, err
}
lastIdx = int(n)

Check failure

Code scanning / CodeQL

Incorrect conversion between integer types High

Incorrect conversion of a signed 64-bit integer from
strconv.ParseInt
to a lower bit size type int without an upper bound check.

Copilot Autofix

AI 19 days ago

In general, the issue arises because strconv.ParseInt is invoked with a 64-bit size, but the result is then converted to int with no check. To fix this pattern, either (a) parse with a bit size matching the target type (e.g., 32 for 32-bit int), or (b) keep parsing as 64-bit and add explicit bounds checks to ensure the value fits into the target type before converting.

Here, lastIdx is an int used as an index-like counter derived from the numeric suffix of an endpoint name. The safest and simplest change that preserves behavior is to (1) parse the numeric suffix as a 32-bit integer (since int is at least 32 bits) using strconv.ParseInt with bit size 32, (2) check that the parsed value is non-negative and within the int range, and then (3) convert to int. Because we must avoid extra imports and math is not currently imported in this file, we cannot rely on math.MaxInt32 directly; however, if we parse with bit size 32 and then convert to int, the value is guaranteed to fit in int on 32-bit and 64-bit Go architectures (where int is 32 or 64 bits respectively). We should also guard against negative values defensively, even though the regex extracts only digits.

Concretely, in addSingleEndpoint in src/runtime/virtcontainers/network_linux.go, modify the block that extracts and parses the numeric suffix: change the ParseInt call to use bit size 32, and replace lastIdx = int(n) with a small validity check, e.g., ensure n >= 0 before assigning and otherwise return an error. All other behavior remains the same.

Suggested changeset 1
src/runtime/virtcontainers/network_linux.go

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/runtime/virtcontainers/network_linux.go b/src/runtime/virtcontainers/network_linux.go
--- a/src/runtime/virtcontainers/network_linux.go
+++ b/src/runtime/virtcontainers/network_linux.go
@@ -134,10 +134,13 @@
 		lastEndpoint := n.eps[len(n.eps)-1]
 		re := regexp.MustCompile("[0-9]+")
 		matchStr := re.FindString(lastEndpoint.Name())
-		n, err := strconv.ParseInt(matchStr, 10, 64)
+		n, err := strconv.ParseInt(matchStr, 10, 32)
 		if err != nil {
 			return nil, err
 		}
+		if n < 0 {
+			return nil, fmt.Errorf("invalid endpoint index parsed from name %q: %d", lastEndpoint.Name(), n)
+		}
 		lastIdx = int(n)
 	}
 	if idx <= lastIdx {
EOF
@@ -134,10 +134,13 @@
lastEndpoint := n.eps[len(n.eps)-1]
re := regexp.MustCompile("[0-9]+")
matchStr := re.FindString(lastEndpoint.Name())
n, err := strconv.ParseInt(matchStr, 10, 64)
n, err := strconv.ParseInt(matchStr, 10, 32)
if err != nil {
return nil, err
}
if n < 0 {
return nil, fmt.Errorf("invalid endpoint index parsed from name %q: %d", lastEndpoint.Name(), n)
}
lastIdx = int(n)
}
if idx <= lastIdx {
Copilot is powered by AI and may make mistakes. Always verify output.
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.

4 participants