Skip to content

Commit

Permalink
Merge pull request #59 from trailofbits/go-lock-fixes
Browse files Browse the repository at this point in the history
Reduce FPs in `missing-runlock-on-rwmutex` and `missing-unlock-before-return`
  • Loading branch information
Vasco-jofra authored Apr 3, 2024
2 parents f182941 + b7ed449 commit 772f68e
Show file tree
Hide file tree
Showing 4 changed files with 198 additions and 2 deletions.
85 changes: 85 additions & 0 deletions go/missing-runlock-on-rwmutex.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,14 @@ type RWContainer struct {
counters map[string]int
}

type Fridge struct {
food int
}

func (f *Fridge) RLock() {
fmt.Println(f.food)
}

func main() {
c := RWContainer{
counters: map[string]int{"a": 0, "b": 0},
Expand Down Expand Up @@ -53,6 +61,7 @@ func (c *RWContainer) inc(name string) error {
return fmt.Errorf("letter not allowed")
}
c.mu.RUnlock()
// ok: missing-runlock-on-rwmutex
return nil
}

Expand All @@ -76,6 +85,7 @@ func (c *RWContainer) inc_FP(name string) error {
return fmt.Errorf("letter not allowed")
}
c.mu.RUnlock()
// ok: missing-runlock-on-rwmutex
return nil
}

Expand All @@ -87,3 +97,78 @@ func (c *RWContainer) inc4FP(name string) RUnlocker {
c.mu.Unlock()
}
}

func (c *RWContainer) inc5(name string) error {
f := Fridge{food: 11}
f.RLock()
// ok: missing-runlock-on-rwmutex
return nil
}

func (c *RWContainer) inc6(name string) error {
c.mu.RLock()
c.counters[name]++
defer func() {
fmt.Println("before runlock")
c.mu.RUnlock()
fmt.Println("after runlock")
}()
// ok: missing-runlock-on-rwmutex
return nil
}

func (c *RWContainer) inc6b(name string) error {
c.mu.RLock()
unlocker := c.mu.RUnlock
c.counters[name]++
defer func() {
fmt.Println("before runlock")
unlocker()
fmt.Println("after runlock")
}()
// todook: missing-runlock-on-rwmutex
return nil
}

func (c *RWContainer) inc7(name string) error {
c.mu.RLock()
c.counters[name]++
defer func(earlyExit bool) {
fmt.Println("before runlock")
if (earlyExit) {
// todoruleid: missing-runlock-on-rwmutex
return
}
c.mu.RUnlock()
fmt.Println("after runlock")
}(false)
// ok: missing-runlock-on-rwmutex
return nil
}

func (c *RWContainer) inc8(name string) error {
c.mu.RLock()
c.counters[name]++
_, err := fmt.Println("test if")
if err != nil {
c.mu.RUnlock()
// ok: missing-runlock-on-rwmutex
return nil, err
}
// ruleid: missing-runlock-on-rwmutex
return nil
}

func (c *RWContainer) inc8b(name string) error {
c.mu.RLock()
c.counters[name]++
unlocker := c.mu.RUnlock
_, err := fmt.Println("test if")
if err != nil {
unlocker()
// todook: missing-runlock-on-rwmutex
return nil, err
}
// ruleid: missing-runlock-on-rwmutex
return nil
}
14 changes: 13 additions & 1 deletion go/missing-runlock-on-rwmutex.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
rules:
- id: missing-runlock-on-rwmutex
message: >-
Missing `RUnlock` on an `RWMutex` lock before returning from a function
Missing `RUnlock` on an `RWMutex` (`$T` variable) lock before returning from a function
languages: [go]
severity: ERROR
metadata:
Expand All @@ -21,6 +21,11 @@ rules:
- pattern-either:
- pattern: panic(...)
- pattern: return ...
- metavariable-pattern:
metavariable: $T
patterns:
- pattern: |
($T : sync.RWMutex)
- pattern-inside: |
$T.RLock()
...
Expand All @@ -30,6 +35,13 @@ rules:
- pattern-not-inside: |
defer $T.RUnlock()
...
- pattern-not-inside: |
defer func(...) {
...
$T.RUnlock()
...
}(...)
...
- pattern-not-inside: |
$FOO(..., ..., func(...) {
...
Expand Down
87 changes: 87 additions & 0 deletions go/missing-unlock-before-return.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,14 @@ type Container struct {
counters map[string]int
}

type Fridge struct {
food int
}

func (f *Fridge) Lock() {
fmt.Println(f.food)
}

func main() {
c := Container{
counters: map[string]int{"a": 0, "b": 0},
Expand Down Expand Up @@ -53,6 +61,7 @@ func (c *Container) inc(name string) error {
return fmt.Errorf("letter not allowed")
}
c.mu.Unlock()
// ok: missing-unlock-before-return
return nil
}

Expand All @@ -65,6 +74,7 @@ func (c *Container) inc_FP(name string) error {
return fmt.Errorf("letter not allowed")
}
c.mu.Unlock()
// ok: missing-unlock-before-return
return nil
}

Expand All @@ -78,6 +88,7 @@ func (c *Container) inc2(name string) error {
panic("letter not allowed")
}
c.mu.Unlock()
// ok: missing-unlock-before-return
return nil
}

Expand All @@ -90,6 +101,7 @@ func (c *Container) inc2_FP(name string) error {
panic("letter not allowed")
}
c.mu.Unlock()
// ok: missing-unlock-before-return
return nil
}

Expand All @@ -113,3 +125,78 @@ func (c *Container) inc4FP(name string) Unlocker {
c.mu.Unlock()
}
}

func (c *Container) inc5(name string) error {
f := Fridge{food: 11}
f.Lock()
// ok: missing-unlock-before-return
return nil
}

func (c *Container) inc6(name string) error {
c.mu.Lock()
c.counters[name]++
defer func() {
fmt.Println("before unlock")
c.mu.Unlock()
fmt.Println("after unlock")
}()
// ok: missing-unlock-before-return
return nil
}

func (c *Container) inc6b(name string) error {
c.mu.Lock()
unlocker := c.mu.Unlock
c.counters[name]++
defer func() {
fmt.Println("before unlock")
unlocker()
fmt.Println("after unlock")
}()
// todook: missing-unlock-before-return
return nil
}

func (c *Container) inc7(name string) error {
c.mu.Lock()
c.counters[name]++
defer func(earlyExit bool) {
fmt.Println("before unlock")
if (earlyExit) {
// todoruleid: missing-unlock-before-return
return
}
c.mu.Unlock()
fmt.Println("after unlock")
}(false)
// ok: missing-unlock-before-return
return nil
}

func (c *Container) inc8(name string) error {
c.mu.Lock()
c.counters[name]++
_, err := fmt.Println("test if")
if err != nil {
c.mu.Unlock()
// ok: missing-unlock-before-return
return nil, err
}
// ruleid: missing-unlock-before-return
return nil
}

func (c *Container) inc8b(name string) error {
c.mu.Lock()
c.counters[name]++
unlocker := c.mu.Unlock
_, err := fmt.Println("test if")
if err != nil {
unlocker()
// todook: missing-unlock-before-return
return nil, err
}
// ruleid: missing-unlock-before-return
return nil
}
14 changes: 13 additions & 1 deletion go/missing-unlock-before-return.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
rules:
- id: missing-unlock-before-return
message: >-
Missing mutex unlock before returning from a function.
Missing mutex unlock (`$T` variable) before returning from a function.
This could result in panics resulting from double lock operations
languages: [go]
severity: ERROR
Expand All @@ -22,6 +22,11 @@ rules:
- pattern-either:
- pattern: panic(...)
- pattern: return ...
- metavariable-pattern:
metavariable: $T
patterns:
- pattern: |
($T : sync.Mutex)
- pattern-inside: |
$T.Lock()
...
Expand All @@ -31,6 +36,13 @@ rules:
- pattern-not-inside: |
defer $T.Unlock()
...
- pattern-not-inside: |
defer func(...) {
...
$T.Unlock()
...
}(...)
...
- pattern-not-inside: |
$FOO(..., ..., func(...) {
...
Expand Down

0 comments on commit 772f68e

Please sign in to comment.