From dac77d8a7cc20df35f48b596baa7c2a732f9aa63 Mon Sep 17 00:00:00 2001 From: Kevin Parsons Date: Fri, 19 Apr 2019 10:43:53 -0700 Subject: [PATCH 1/2] etw: Fix panic when provider creation fails There is a cleanup function deferred to remove the provider from the global provider map. However, this function didn't bind the value of provider ahead of time, so when the provider creation returned with nil for provider, we tried to cleanup a nil provider, and panicked. --- pkg/etw/provider.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/etw/provider.go b/pkg/etw/provider.go index 7a3c13d..c30b943 100644 --- a/pkg/etw/provider.go +++ b/pkg/etw/provider.go @@ -130,11 +130,11 @@ func NewProviderWithID(name string, id *guid.GUID, callback EnableCallback) (pro }) provider = providers.newProvider() - defer func() { + defer func(provider *Provider) { if err != nil { providers.removeProvider(provider) } - }() + }(provider) provider.ID = id provider.callback = callback From a4918f0a0ff2072c724ddd6405f7266a3f0743db Mon Sep 17 00:00:00 2001 From: Kevin Parsons Date: Fri, 19 Apr 2019 15:23:39 -0700 Subject: [PATCH 2/2] Make ETW no-op on ARM Proper support for ETW on ARM will require additional changes in Go to support more than 16 bytes of arguments to a syscall callback. Until this work is done, the ETW package is being changed to no-op on ARM. --- pkg/etw/newprovider.go | 53 +++++++++++++++++++++++++++ pkg/etw/newprovider_unsupported.go | 12 ++++++ pkg/etw/provider.go | 59 ++++++++---------------------- 3 files changed, 81 insertions(+), 43 deletions(-) create mode 100644 pkg/etw/newprovider.go create mode 100644 pkg/etw/newprovider_unsupported.go diff --git a/pkg/etw/newprovider.go b/pkg/etw/newprovider.go new file mode 100644 index 0000000..07baba4 --- /dev/null +++ b/pkg/etw/newprovider.go @@ -0,0 +1,53 @@ +// +build amd64 arm64 + +package etw + +import ( + "bytes" + "encoding/binary" + "unsafe" + + "github.com/Microsoft/go-winio/pkg/guid" + "golang.org/x/sys/windows" +) + +// NewProviderWithID creates and registers a new ETW provider, allowing the +// provider ID to be manually specified. This is most useful when there is an +// existing provider ID that must be used to conform to existing diagnostic +// infrastructure. +func NewProviderWithID(name string, id *guid.GUID, callback EnableCallback) (provider *Provider, err error) { + providerCallbackOnce.Do(func() { + globalProviderCallback = windows.NewCallback(providerCallbackAdapter) + }) + + provider = providers.newProvider() + defer func(provider *Provider) { + if err != nil { + providers.removeProvider(provider) + } + }(provider) + provider.ID = id + provider.callback = callback + + if err := eventRegister((*windows.GUID)(provider.ID), globalProviderCallback, uintptr(provider.index), &provider.handle); err != nil { + return nil, err + } + + metadata := &bytes.Buffer{} + binary.Write(metadata, binary.LittleEndian, uint16(0)) // Write empty size for buffer (to update later) + metadata.WriteString(name) + metadata.WriteByte(0) // Null terminator for name + binary.LittleEndian.PutUint16(metadata.Bytes(), uint16(metadata.Len())) // Update the size at the beginning of the buffer + provider.metadata = metadata.Bytes() + + if err := eventSetInformation( + provider.handle, + eventInfoClassProviderSetTraits, + uintptr(unsafe.Pointer(&provider.metadata[0])), + uint32(len(provider.metadata))); err != nil { + + return nil, err + } + + return provider, nil +} diff --git a/pkg/etw/newprovider_unsupported.go b/pkg/etw/newprovider_unsupported.go new file mode 100644 index 0000000..ba1669c --- /dev/null +++ b/pkg/etw/newprovider_unsupported.go @@ -0,0 +1,12 @@ +// +build arm 386 + +package etw + +import ( + "github.com/Microsoft/go-winio/pkg/guid" +) + +// NewProviderWithID returns a nil provider on unsupported platforms. +func NewProviderWithID(name string, id *guid.GUID, callback EnableCallback) (provider *Provider, err error) { + return nil, nil +} diff --git a/pkg/etw/provider.go b/pkg/etw/provider.go index c30b943..a50835c 100644 --- a/pkg/etw/provider.go +++ b/pkg/etw/provider.go @@ -1,12 +1,10 @@ package etw import ( - "bytes" "crypto/sha1" "encoding/binary" "strings" "unicode/utf16" - "unsafe" "github.com/Microsoft/go-winio/pkg/guid" "golang.org/x/sys/windows" @@ -29,6 +27,10 @@ type Provider struct { // String returns the `provider`.ID as a string func (provider *Provider) String() string { + if provider == nil { + return "" + } + return provider.ID.String() } @@ -120,49 +122,12 @@ func NewProvider(name string, callback EnableCallback) (provider *Provider, err return NewProviderWithID(name, providerIDFromName(name), callback) } -// NewProviderWithID creates and registers a new ETW provider, allowing the -// provider ID to be manually specified. This is most useful when there is an -// existing provider ID that must be used to conform to existing diagnostic -// infrastructure. -func NewProviderWithID(name string, id *guid.GUID, callback EnableCallback) (provider *Provider, err error) { - providerCallbackOnce.Do(func() { - globalProviderCallback = windows.NewCallback(providerCallbackAdapter) - }) - - provider = providers.newProvider() - defer func(provider *Provider) { - if err != nil { - providers.removeProvider(provider) - } - }(provider) - provider.ID = id - provider.callback = callback - - if err := eventRegister((*windows.GUID)(provider.ID), globalProviderCallback, uintptr(provider.index), &provider.handle); err != nil { - return nil, err - } - - metadata := &bytes.Buffer{} - binary.Write(metadata, binary.LittleEndian, uint16(0)) // Write empty size for buffer (to update later) - metadata.WriteString(name) - metadata.WriteByte(0) // Null terminator for name - binary.LittleEndian.PutUint16(metadata.Bytes(), uint16(metadata.Len())) // Update the size at the beginning of the buffer - provider.metadata = metadata.Bytes() - - if err := eventSetInformation( - provider.handle, - eventInfoClassProviderSetTraits, - uintptr(unsafe.Pointer(&provider.metadata[0])), - uint32(len(provider.metadata))); err != nil { - - return nil, err - } - - return provider, nil -} - // Close unregisters the provider. func (provider *Provider) Close() error { + if provider == nil { + return nil + } + providers.removeProvider(provider) return eventUnregister(provider.handle) } @@ -185,6 +150,10 @@ func (provider *Provider) IsEnabledForLevel(level Level) bool { // infrastructure, it can be useful to check if an event will actually be // consumed before doing expensive work to build the event data. func (provider *Provider) IsEnabledForLevelAndKeywords(level Level, keywords uint64) bool { + if provider == nil { + return false + } + if !provider.enabled { return false } @@ -206,6 +175,10 @@ func (provider *Provider) IsEnabledForLevelAndKeywords(level Level, keywords uin // constructed based on the EventOpt and FieldOpt values that are passed as // opts. func (provider *Provider) WriteEvent(name string, eventOpts []EventOpt, fieldOpts []FieldOpt) error { + if provider == nil { + return nil + } + options := eventOptions{descriptor: newEventDescriptor()} em := &eventMetadata{} ed := &eventData{}