Skip to content

Commit

Permalink
Merge pull request opensearch-project#189 from parca-dev/rm_sec_checks
Browse files Browse the repository at this point in the history
  • Loading branch information
@kakkoyun
kakkoyun authored Jan 5, 2022
2 parents cf792b1 + 21f5d84 commit d20ec7a
Show file tree
Hide file tree
Showing 3 changed files with 101 additions and 71 deletions.
6 changes: 5 additions & 1 deletion cmd/debug-info/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,9 @@ func main() {
})
case "extract <path>":
g.Add(func() error {
if err := os.RemoveAll(flags.Extract.OutputDir); err != nil {
return fmt.Errorf("failed to clean output dir, %s: %w", flags.Extract.OutputDir, err)
}
if err := os.MkdirAll(flags.Extract.OutputDir, 0755); err != nil {
return fmt.Errorf("failed to create output dir, %s: %w", flags.Extract.OutputDir, err)
}
Expand All @@ -133,10 +136,11 @@ func main() {
}

for _, f := range files {
// ./out/<buildid>/debuginfo
_, p := filepath.Split(filepath.Dir(f))
output := filepath.Join(flags.Extract.OutputDir, filepath.Base(p))
if err := os.Rename(f, output); err != nil {
level.Error(logger).Log("msg", "failed to move file", "file", output)
level.Error(logger).Log("msg", "failed to move file", "file", output, "err", err)
continue
}
level.Info(logger).Log("msg", "debug information extracted", "file", output)
Expand Down
10 changes: 2 additions & 8 deletions pkg/agent/profile.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"io"
"os"
"runtime"
"strings"
"sync"
"time"
"unsafe"
Expand All @@ -33,21 +34,16 @@ import (
"github.com/go-kit/log/level"
"github.com/google/pprof/profile"
profilestorepb "github.com/parca-dev/parca/gen/proto/go/parca/profilestore/v1alpha1"
"github.com/prometheus/common/model"
"golang.org/x/sys/unix"
"google.golang.org/grpc"

"github.com/parca-dev/parca-agent/pkg/byteorder"
"github.com/parca-dev/parca-agent/pkg/debuginfo"

"github.com/parca-dev/parca-agent/pkg/ksym"
"github.com/parca-dev/parca-agent/pkg/maps"
"github.com/parca-dev/parca-agent/pkg/perf"
)
import (
"strings"

"github.com/prometheus/common/model"
)

//go:embed parca-agent.bpf.o
var bpfObj []byte
Expand Down Expand Up @@ -485,12 +481,10 @@ func (p *CgroupProfiler) profileLoop(ctx context.Context, now time.Time, counts,
var labeloldformat []*profilestorepb.Label

for key, value := range labels {

labeloldformat = append(labeloldformat,
&profilestorepb.Label{Name: string(key),
Value: string(value),
})

}

_, err = p.writeClient.WriteRaw(ctx, &profilestorepb.WriteRawRequest{
Expand Down
156 changes: 94 additions & 62 deletions pkg/debuginfo/debuginfo.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -243,29 +243,28 @@ func (di *Extractor) extract(ctx context.Context, buildID string, file string) (
level.Debug(di.logger).Log("msg", "failed to determine if binary is a Go binary", "path", file, "err", err)
}

var (
cmd *exec.Cmd
debugInfoFile string
)
toRemove, err := sectionsToRemove(file)
if err != nil {
level.Debug(di.logger).Log("msg", "failed to determine sections to remove", "path", file, "err", err)
}

var debugInfoFile string
switch {
case hasDWARF:
cmd, debugInfoFile = di.useStrip(ctx, tmpDir, file)
debugInfoFile, err = di.useStrip(ctx, tmpDir, file, toRemove)
case isGo:
cmd, debugInfoFile = di.useObjcopy(ctx, tmpDir, file)
debugInfoFile, err = di.useObjcopy(ctx, tmpDir, file, toRemove)
default:
cmd, debugInfoFile = di.useStrip(ctx, tmpDir, file)
debugInfoFile, err = di.useStrip(ctx, tmpDir, file, toRemove)
}
if out, err := cmd.CombinedOutput(); err != nil {
level.Error(di.logger).Log(
"msg", "external binutils command call failed",
"output", strings.ReplaceAll(string(out), "\n", ""),
"file", file,
)
return "", fmt.Errorf("failed to extract debug information from binary: %w", err)
if err != nil {
level.Error(di.logger).Log("msg", "external command call failed", "err", err, "file", file)
return "", err
}

// Check if the debug information file is actually created.
if exists, err := exists(debugInfoFile); !exists {
const msg = "external binutils command failed to extract debug information from binary"
const msg = "external binary utility command failed to extract debug information from binary"
if err != nil {
return "", fmt.Errorf(msg+": %w", err)
}
Expand All @@ -274,44 +273,61 @@ func (di *Extractor) extract(ctx context.Context, buildID string, file string) (
return debugInfoFile, nil
}

func (di *Extractor) useStrip(ctx context.Context, dir string, file string) (*exec.Cmd, string) {
func (di *Extractor) useStrip(ctx context.Context, dir string, file string, toRemove []string) (string, error) {
level.Debug(di.logger).Log("msg", "using eu-strip", "file", file)
// Extract debug symbols.
// If we have DWARF symbols, they are enough for us to symbolize the profiles.
// We observed that having DWARF debug symbols and symbol table together caused us problem in certain cases.
// As DWARF symbols enough on their own we just extract those.
// eu-strip --strip-debug extracts the .debug/.zdebug sections from the object files.
args := []string{"--strip-debug"}
for _, s := range toRemove {
args = append(args, "--remove-section", s)
}
debugInfoFile := path.Join(dir, "debuginfo")
interimFile := path.Join(dir, "binary.stripped")
cmd := exec.CommandContext(ctx,
"eu-strip", "--strip-debug",
"--remove-section", ".debug_gdb_scripts", // causes some trouble when it's set to SHT_NOBITS
"-f", debugInfoFile,
"-o", interimFile,
file)
defer func() {
os.Remove(interimFile)
}()
return cmd, debugInfoFile
args = append(args, "-f", debugInfoFile, "-o", interimFile, file)
cmd := exec.CommandContext(ctx, "eu-strip", args...)
defer os.Remove(interimFile)

if out, err := cmd.CombinedOutput(); err != nil {
level.Debug(di.logger).Log(
"msg", "external eu-strip command call failed",
"output", strings.ReplaceAll(string(out), "\n", ""),
"file", file,
)
return "", fmt.Errorf("failed to extract debug information from binary: %w", err)
}

return debugInfoFile, nil
}

func (di *Extractor) useObjcopy(ctx context.Context, dir string, file string) (*exec.Cmd, string) {
debugInfoFile := path.Join(dir, "debuginfo")
func (di *Extractor) useObjcopy(ctx context.Context, dir string, file string, toRemove []string) (string, error) {
level.Debug(di.logger).Log("msg", "using objcopy", "file", file)
// Go binaries has a special case. They use ".gopclntab" section to symbolize addresses.
// We need to keep ".note.go.buildid", ".symtab" and ".gopclntab",
// however it doesn't hurt to keep rather small sections.
return exec.CommandContext(ctx,
"objcopy",
// NOTICE: Keep debug information till we find a better for symbolizing Go binaries without DWARF.
//"-R", ".zdebug_*",
//"-R", ".debug_*",
"--remove-section", ".text", // executable
"--remove-section", ".rodata*", // constants
"--remove-section", ".debug_gdb_scripts", // causes some trouble when it's set to SHT_NOBITS
args := []string{}
toRemove = append(toRemove, ".text", ".rodata*")
for _, s := range toRemove {
args = append(args, "--remove-section", s)
}
debugInfoFile := path.Join(dir, "debuginfo")
args = append(args,
file, // source
debugInfoFile, // destination
), debugInfoFile
)
cmd := exec.CommandContext(ctx, "objcopy", args...)
if out, err := cmd.CombinedOutput(); err != nil {
level.Debug(di.logger).Log(
"msg", "external objcopy command call failed",
"output", strings.ReplaceAll(string(out), "\n", ""),
"file", file,
)
return "", fmt.Errorf("failed to extract debug information from binary: %w", err)
}

return debugInfoFile, nil
}

func (di *Extractor) uploadDebugInfo(ctx context.Context, buildID string, file string) error {
Expand Down Expand Up @@ -347,49 +363,49 @@ func hasDebugInfo(path string) (bool, error) {
}

func hasDWARF(path string) (bool, error) {
exe, err := elf.Open(path)
f, err := elf.Open(path)
if err != nil {
return false, fmt.Errorf("failed to open elf: %w", err)
}
defer exe.Close()
defer f.Close()

data, err := getDWARF(exe)
data, err := dwarf(f)
if err != nil {
return false, fmt.Errorf("failed to read DWARF sections: %w", err)
}

return len(data) > 0, nil
}

// A simplified and modified version of debug/elf.DWARF().
func getDWARF(f *elf.File) (map[string][]byte, error) {
dwarfSuffix := func(s *elf.Section) string {
switch {
case strings.HasPrefix(s.Name, ".debug_"):
return s.Name[7:]
case strings.HasPrefix(s.Name, ".zdebug_"):
return s.Name[8:]
case strings.HasPrefix(s.Name, "__debug_"): // macos
return s.Name[8:]
default:
return ""
}
var dwarfSuffix = func(s *elf.Section) string {
switch {
case strings.HasPrefix(s.Name, ".debug_"):
return s.Name[7:]
case strings.HasPrefix(s.Name, ".zdebug_"):
return s.Name[8:]
case strings.HasPrefix(s.Name, "__debug_"): // macos
return s.Name[8:]
default:
return ""
}
}

// A simplified and modified version of debug/elf.DWARF().
func dwarf(f *elf.File) (map[string][]byte, error) {
// There are many DWARf sections, but these are the ones
// the debug/dwarf package started with "abbrev", "info", "str", "line", "ranges".
// Possible canditates for future: "loc", "loclists", "rnglists"
sections := map[string]*string{"abbrev": nil, "info": nil, "str": nil, "line": nil, "ranges": nil}
data := map[string][]byte{}
for _, s := range f.Sections {
suffix := dwarfSuffix(s)
for _, sec := range f.Sections {
suffix := dwarfSuffix(sec)
if suffix == "" {
continue
}
if _, ok := sections[suffix]; !ok {
continue
}
b, err := s.Data()
b, err := sec.Data()
if err != nil {
return nil, fmt.Errorf("failed to read debug section: %w", err)
}
Expand All @@ -399,24 +415,40 @@ func getDWARF(f *elf.File) (map[string][]byte, error) {
return data, nil
}

func sectionsToRemove(path string) ([]string, error) {
var sections []string
f, err := elf.Open(path)
if err != nil {
return sections, fmt.Errorf("failed to open elf file: %w", err)
}
defer f.Close()

for _, sec := range f.Sections {
if dwarfSuffix(sec) != "" && sec.Type == elf.SHT_NOBITS { // causes some trouble when it's set to SHT_NOBITS
sections = append(sections, sec.Name)
}
}
return sections, nil
}

func isSymbolizableGoBinary(path string) (bool, error) {
// Checks ".note.go.buildid" section and symtab better to keep those sections in object file.
exe, err := elf.Open(path)
f, err := elf.Open(path)
if err != nil {
return false, fmt.Errorf("failed to open elf: %w", err)
}
defer exe.Close()
defer f.Close()

isGo := false
for _, s := range exe.Sections {
if s.Name == ".note.go.buildid" {
for _, sec := range f.Sections {
if sec.Name == ".note.go.buildid" {
isGo = true
}
}

// In case ".note.go.buildid" section is stripped, check for symbols.
if !isGo {
syms, err := exe.Symbols()
syms, err := f.Symbols()
if err != nil {
return false, fmt.Errorf("failed to read symbols: %w", err)
}
Expand All @@ -438,7 +470,7 @@ func isSymbolizableGoBinary(path string) (bool, error) {
// Check if the Go binary symbolizable.
// Go binaries has a special case. They use ".gopclntab" section to symbolize addresses.
var pclntab []byte
if sec := exe.Section(".gopclntab"); sec != nil {
if sec := f.Section(".gopclntab"); sec != nil {
pclntab, err = sec.Data()
if err != nil {
return false, fmt.Errorf("could not find .gopclntab section: %w", err)
Expand Down

0 comments on commit d20ec7a

Please sign in to comment.