Skip to content

fix: replace dead len > 2 guard with len != 2 in addSysctl#6883

Open
immanuwell wants to merge 1 commit into
podman-container-tools:mainfrom
immanuwell:fix/sysctl-splitn-dead-check
Open

fix: replace dead len > 2 guard with len != 2 in addSysctl#6883
immanuwell wants to merge 1 commit into
podman-container-tools:mainfrom
immanuwell:fix/sysctl-splitn-dead-check

Conversation

@immanuwell
Copy link
Copy Markdown

@immanuwell immanuwell commented May 29, 2026

What type of PR is this?

/kind bug

What this PR does / why we need it:

strings.SplitN(s, "=", 2) returns at most 2 elements, so len(splitn) > 2 in addSysctl is dead code that never fires. When a sysctl in containers.conf has no =, splitn has length 1 and splitn[1] panics instead of hitting the error return right below.

Changing > 2 to != 2 kills the dead code and catches the bad input.

How to verify it

# ~/.config/containers/containers.conf
[containers]
default_sysctls = ["net.core.rmem_max"]

Before: buildah run panics with index out of range [1] with length 1
After: returns sysctl "net.core.rmem_max" defined in containers.conf must be formatted name=value

Which issue(s) this PR fixes:

None

Special notes for your reviewer:

Does this PR introduce a user-facing change?

None

strings.SplitN(s, "=", 2) returns at most 2 elements, so the
existing len > 2 check never fires. When a sysctl in containers.conf
has no '=' the slice has length 1, and the unconditional access to
splitn[1] panics. Change the guard to != 2 so a malformed sysctl
returns the descriptive error instead of an index-out-of-bounds panic.

Signed-off-by: Immanuel Tikhonov <pchpr.00@list.ru>
Signed-off-by: immanuwell <pchpr.00@list.ru>
Copy link
Copy Markdown
Contributor

@nalind nalind left a comment

Choose a reason for hiding this comment

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

The original version was probably intended to be <, but this works, too.
LGTM, thanks!

@TomSweeneyRedHat
Copy link
Copy Markdown
Contributor

LGTM

@nalind nalind enabled auto-merge May 29, 2026 18:59
@nalind nalind disabled auto-merge May 29, 2026 19:55
@TomSweeneyRedHat
Copy link
Copy Markdown
Contributor

/lgtm

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants