Skip to content

fix: avoid CGO getgrgid_r segfault in static Linux binaries #433

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

Open
nancysangani wants to merge 1 commit into
base: main
Choose a base branch
from
+38 −8
Open

fix: avoid CGO getgrgid_r segfault in static Linux binaries #433

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/release.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ jobs:
GOARCH: ${{ matrix.goarch }}
run: |
go build \
-tags "static system_libgit2 enable_libgit2" \
-tags "static system_libgit2 enable_libgit2 osusergo netgo" \
-ldflags "-X github.com/modelpack/modctl/pkg/version.GitVersion=${{ github.ref_name }} \
-X github.com/modelpack/modctl/pkg/version.GitCommit=$(git rev-parse --short HEAD) \
-X github.com/modelpack/modctl/pkg/version.BuildTime=$(date -u +'%Y-%m-%dT%H:%M:%SZ') \
Expand Down Expand Up @@ -155,4 +155,4 @@ jobs:
checksums.txt
generate_release_notes: true
env:
GITHUB_TOKEN: ${{ secrets.RELEASE_TOKEN }}
GITHUB_TOKEN: ${{ secrets.RELEASE_TOKEN }}
42 changes: 36 additions & 6 deletions pkg/archiver/archiver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,39 @@ import (
"os"
"path/filepath"
"strings"
"syscall"
)

// buildTarHeader creates a tar header from FileInfo without using CGO-based
// os/user.LookupGroupId or os/user.LookupUserId. This avoids a segfault in
// statically linked CGO binaries caused by glibc NSS (getgrgid_r) being
// incompatible with static linking across different glibc versions.
// See: https://github.com/modelpack/modctl/issues/285
func buildTarHeader(info os.FileInfo) (*tar.Header, error) {
header := &tar.Header{
Name: info.Name(),
Size: info.Size(),
Mode: int64(info.Mode()),
ModTime: info.ModTime(),
}

// Set file type flag.
if info.IsDir() {
header.Typeflag = tar.TypeDir
} else {
header.Typeflag = tar.TypeReg
}

// Safely extract UID/GID from syscall.Stat_t without CGO user/group name lookup.
// We intentionally leave Uname/Gname empty to avoid os/user CGO calls entirely.
if stat, ok := info.Sys().(*syscall.Stat_t); ok {
header.Uid = int(stat.Uid)
header.Gid = int(stat.Gid)
}

return header, nil
}
Comment on lines +34 to +57
Copy link

@gemini-code-assist gemini-code-assist bot Feb 27, 2026

Choose a reason for hiding this comment

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

high

The new buildTarHeader function is a good approach to avoid CGO issues. However, it has a couple of issues that represent a regression from using tar.FileInfoHeader:

  • It doesn't handle symbolic links; they are incorrectly treated as regular files.
  • The size for directories is not set to 0. info.Size() for a directory is system-dependent and typically non-zero, but tar headers for directories should have a size of 0.

To address this and make the function more robust, I suggest modifying it to handle symlinks correctly and set sizes properly for different file types. This will require changing the function signature to accept the file path, which is needed to read a symlink's target. You will also need to update the call sites accordingly (see my other comments).

func buildTarHeader(path string, info os.FileInfo) (*tar.Header, error) {
	header := &tar.Header{
		Name:    info.Name(),
		Mode:    int64(info.Mode()),
		ModTime: info.ModTime(),
	}

	if info.Mode()&os.ModeSymlink != 0 {
		header.Typeflag = tar.TypeSymlink
		link, err := os.Readlink(path)
		if err != nil {
			return nil, fmt.Errorf("failed to read symlink %s: %w", path, err)
		}
		header.Linkname = link
	} else if info.IsDir() {
		header.Typeflag = tar.TypeDir
	} else {
		header.Typeflag = tar.TypeReg
		header.Size = info.Size()
	}

	// Safely extract UID/GID from syscall.Stat_t without CGO user/group name lookup.
	// We intentionally leave Uname/Gname empty to avoid os/user CGO calls entirely.
	if stat, ok := info.Sys().(*syscall.Stat_t); ok {
		header.Uid = int(stat.Uid)
		header.Gid = int(stat.Gid)
	}

	return header, nil
}


// Tar creates a tar archive of the specified path (file or directory)
// and returns the content as a stream. For individual files, it preserves
// the directory structure relative to the working directory.
Expand Down Expand Up @@ -56,7 +87,7 @@ func Tar(srcPath string, workDir string) (io.Reader, error) {
return fmt.Errorf("failed to get relative path: %w", err)
}

header, err := tar.FileInfoHeader(info, "")
header, err := buildTarHeader(info)
Copy link

@gemini-code-assist gemini-code-assist bot Feb 27, 2026

Choose a reason for hiding this comment

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

high

To support the proposed changes in buildTarHeader (which now requires the file path to handle symlinks correctly), please update this call to pass the path variable.

				header, err := buildTarHeader(path, info)

if err != nil {
return fmt.Errorf("failed to create tar header: %w", err)
}
Expand Down Expand Up @@ -95,14 +126,13 @@ func Tar(srcPath string, workDir string) (io.Reader, error) {
}
defer file.Close()

header, err := tar.FileInfoHeader(info, "")
header, err := buildTarHeader(info)
Copy link

@gemini-code-assist gemini-code-assist bot Feb 27, 2026

Choose a reason for hiding this comment

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

high

To support the proposed changes in buildTarHeader (which now requires the file path to handle symlinks correctly), please update this call to pass the srcPath variable.

			header, err := buildTarHeader(srcPath, info)

if err != nil {
pw.CloseWithError(fmt.Errorf("failed to create tar header: %w", err))
return
}

// Use relative path as the header name to preserve directory structure
// This keeps the directory structure as part of the file path in the tar.
// Use relative path as the header name to preserve directory structure.
relPath, err := filepath.Rel(workDir, srcPath)
if err != nil {
pw.CloseWithError(fmt.Errorf("failed to get relative path: %w", err))
Expand Down Expand Up @@ -189,9 +219,9 @@ func Untar(reader io.Reader, destPath string) error {
}
file.Close()

// Set correct permissions for the directory.
// Set correct permissions for the file.
if err := os.Chmod(targetPath, os.FileMode(header.Mode)); err != nil {
return fmt.Errorf("failed to set directory permissions %s: %w", targetPath, err)
return fmt.Errorf("failed to set file permissions %s: %w", targetPath, err)
}
// Set modification time for the file.
if err := os.Chtimes(targetPath, header.ModTime, header.ModTime); err != nil {
Expand Down