-
Notifications
You must be signed in to change notification settings - Fork 29
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.
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 | ||
|
nancysangani marked this conversation as resolved.
Outdated
|
||
| } | ||
|
|
||
| // 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 | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The new
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. | ||
|
|
@@ -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) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| if err != nil { | ||
| return fmt.Errorf("failed to create tar header: %w", err) | ||
| } | ||
|
|
@@ -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) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| 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)) | ||
|
|
@@ -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 { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.