Do not unconditionally allocate in kernel.FDTable.setAll().

`slice := *(*[]unsafe.Pointer)(...)` makes a copy of the slice header, which
then escapes because of the conditional `atomic.StorePointer(&f.slice, &slice)`
from table expansion. This occurs even when the table doesn't expand, and when
it can't (e.g. `close()` => `f.setAll(nil)`). Fix this by avoiding the copy
until after table expansion.

Before this CL:
```
TEXT pkg/sentry/kernel/kernel.(*FDTable).setAll(SB) pkg/sentry/kernel/fd_table_unsafe.go
  fd_table_unsafe.go:119        0x7f00005f50e0          64488b0c25f8ffffff              MOVQ FS:0xfffffff8, CX
  fd_table_unsafe.go:119        0x7f00005f50e9          483b6110                        CMPQ 0x10(CX), SP
  fd_table_unsafe.go:119        0x7f00005f50ed          0f864d040000                    JBE 0x7f00005f5540
  fd_table_unsafe.go:119        0x7f00005f50f3          4883c480                        ADDQ $-0x80, SP
  fd_table_unsafe.go:119        0x7f00005f50f7          48896c2478                      MOVQ BP, 0x78(SP)
  fd_table_unsafe.go:119        0x7f00005f50fc          488d6c2478                      LEAQ 0x78(SP), BP
  fd_table_unsafe.go:120        0x7f00005f5101          488b8424a8000000                MOVQ 0xa8(SP), AX
  fd_table_unsafe.go:120        0x7f00005f5109          4885c0                          TESTQ AX, AX
  fd_table_unsafe.go:120        0x7f00005f510c          7411                            JE 0x7f00005f511f
  fd_table_unsafe.go:120        0x7f00005f510e          488b8c24b0000000                MOVQ 0xb0(SP), CX
  fd_table_unsafe.go:120        0x7f00005f5116          4885c9                          TESTQ CX, CX
  fd_table_unsafe.go:120        0x7f00005f5119          0f8500040000                    JNE 0x7f00005f551f
  fd_table_unsafe.go:124        0x7f00005f511f          488d05da115700                  LEAQ 0x5711da(IP), AX
  fd_table_unsafe.go:124        0x7f00005f5126          48890424                        MOVQ AX, 0(SP)
  fd_table_unsafe.go:124        0x7f00005f512a          e8d19fa1ff                      CALL runtime.newobject(SB)
  fd_table_unsafe.go:124        0x7f00005f512f          488b7c2408                      MOVQ 0x8(SP), DI
  fd_table_unsafe.go:124        0x7f00005f5134          488b842488000000                MOVQ 0x88(SP), AX
  fd_table_unsafe.go:124        0x7f00005f513c          488b4820                        MOVQ 0x20(AX), CX
  fd_table_unsafe.go:124        0x7f00005f5140          488b5108                        MOVQ 0x8(CX), DX
  fd_table_unsafe.go:124        0x7f00005f5144          488b19                          MOVQ 0(CX), BX
  fd_table_unsafe.go:124        0x7f00005f5147          488b4910                        MOVQ 0x10(CX), CX
  fd_table_unsafe.go:124        0x7f00005f514b          48895708                        MOVQ DX, 0x8(DI)
  fd_table_unsafe.go:124        0x7f00005f514f          48894f10                        MOVQ CX, 0x10(DI)
  fd_table_unsafe.go:124        0x7f00005f5153          833df6e1120100                  CMPL $0x0, runtime.writeBarrier(SB)
  fd_table_unsafe.go:124        0x7f00005f515a          660f1f440000                    NOPW 0(AX)(AX*1)
  fd_table_unsafe.go:124        0x7f00005f5160          0f8589030000                    JNE 0x7f00005f54ef
  fd_table_unsafe.go:124        0x7f00005f5166          48891f                          MOVQ BX, 0(DI)
  fd_table_unsafe.go:124        0x7f00005f5169          48897c2470                      MOVQ DI, 0x70(SP)
  fd_table_unsafe.go:127        0x7f00005f516e          8bb424a0000000                  MOVL 0xa0(SP), SI
  fd_table_unsafe.go:127        0x7f00005f5175          39d6                            CMPL DX, SI
  fd_table_unsafe.go:127        0x7f00005f5177          0f8c5f030000                    JL 0x7f00005f54dc
  ...
```

After this CL:
```
TEXT pkg/sentry/kernel/kernel.(*FDTable).setAll(SB) pkg/sentry/kernel/fd_table_unsafe.go
  fd_table_unsafe.go:119        0x7f00005f50e0          64488b0c25f8ffffff              MOVQ FS:0xfffffff8, CX
  fd_table_unsafe.go:119        0x7f00005f50e9          488d4424e8                      LEAQ -0x18(SP), AX
  fd_table_unsafe.go:119        0x7f00005f50ee          483b4110                        CMPQ 0x10(CX), AX
  fd_table_unsafe.go:119        0x7f00005f50f2          0f868e040000                    JBE 0x7f00005f5586
  fd_table_unsafe.go:119        0x7f00005f50f8          4881ec98000000                  SUBQ $0x98, SP
  fd_table_unsafe.go:119        0x7f00005f50ff          4889ac2490000000                MOVQ BP, 0x90(SP)
  fd_table_unsafe.go:119        0x7f00005f5107          488dac2490000000                LEAQ 0x90(SP), BP
  fd_table_unsafe.go:120        0x7f00005f510f          488b9424c0000000                MOVQ 0xc0(SP), DX
  fd_table_unsafe.go:120        0x7f00005f5117          660f1f840000000000              NOPW 0(AX)(AX*1)
  fd_table_unsafe.go:120        0x7f00005f5120          4885d2                          TESTQ DX, DX
  fd_table_unsafe.go:120        0x7f00005f5123          0f8406040000                    JE 0x7f00005f552f
  fd_table_unsafe.go:120        0x7f00005f5129          488b9c24c8000000                MOVQ 0xc8(SP), BX
  fd_table_unsafe.go:120        0x7f00005f5131          4885db                          TESTQ BX, BX
  fd_table_unsafe.go:120        0x7f00005f5134          0f852b040000                    JNE 0x7f00005f5565
  fd_table_unsafe.go:124        0x7f00005f513a          488bb424a0000000                MOVQ 0xa0(SP), SI
  fd_table_unsafe.go:124        0x7f00005f5142          488b7e20                        MOVQ 0x20(SI), DI
  fd_table_unsafe.go:127        0x7f00005f5146          4c8b4708                        MOVQ 0x8(DI), R8
  fd_table_unsafe.go:127        0x7f00005f514a          448b8c24b8000000                MOVL 0xb8(SP), R9
  fd_table_unsafe.go:127        0x7f00005f5152          4539c1                          CMPL R8, R9
  fd_table_unsafe.go:127        0x7f00005f5155          0f8d4a020000                    JGE 0x7f00005f53a5
  ...
```

PiperOrigin-RevId: 345363242
This commit is contained in:
Jamie Liu 2020-12-02 19:38:49 -08:00 committed by gVisor bot
parent 8b692f5931
commit f559db5690
1 changed files with 7 additions and 4 deletions

View File

@ -121,18 +121,21 @@ func (f *FDTable) setAll(ctx context.Context, fd int32, file *fs.File, fileVFS2
panic("VFS1 and VFS2 files set")
}
slice := *(*[]unsafe.Pointer)(atomic.LoadPointer(&f.slice))
slicePtr := (*[]unsafe.Pointer)(atomic.LoadPointer(&f.slice))
// Grow the table as required.
if last := int32(len(slice)); fd >= last {
if last := int32(len(*slicePtr)); fd >= last {
end := fd + 1
if end < 2*last {
end = 2 * last
}
slice = append(slice, make([]unsafe.Pointer, end-last)...)
atomic.StorePointer(&f.slice, unsafe.Pointer(&slice))
newSlice := append(*slicePtr, make([]unsafe.Pointer, end-last)...)
slicePtr = &newSlice
atomic.StorePointer(&f.slice, unsafe.Pointer(slicePtr))
}
slice := *slicePtr
var desc *descriptor
if file != nil || fileVFS2 != nil {
desc = &descriptor{