fix: handle overwriting tags in syslinux ADV

This is (still) being used in Talos to handle upgrade rollbacks.

There were multiple problems with this code, and one of them leads to
panic if the tag is written multiple times without deletion:

```
github.com/siderolabs/talos/internal/app/machined/pkg/runtime/v1alpha1/bootloader/adv/syslinux.ADV.SetTagBytes({0xc00175bc00?, 0x1f11dbe?, 0xed4f4d?}, 0x0?, {0xc000afb7f0?, 0x400?, 0x0?})
/src/internal/app/machined/pkg/runtime/v1alpha1/bootloader/adv/syslinux/syslinux.go:125 +0x270
github.com/siderolabs/talos/internal/app/machined/pkg/runtime/v1alpha1/bootloader/adv/syslinux.ADV.SetTag(...)
/src/internal/app/machined/pkg/runtime/v1alpha1/bootloader/adv/syslinux/syslinux.go:95
github.com/siderolabs/talos/cmd/installer/pkg/install.(*Installer).Install(0xc0004374a0, 0x5)
/src/cmd/installer/pkg/install/install.go
```

The `uint8()` conversion was causing overflow and wrong index when ADV
real length is over 255.

Fix multiple writes of the same tag by deleting previous value first.

Signed-off-by: Andrey Smirnov <andrey.smirnov@talos-systems.com>
This commit is contained in:
Andrey Smirnov 2023-01-17 23:21:39 +04:00
parent 70d9428a1d
commit 0b65bbfc87
No known key found for this signature in database
GPG Key ID: 7B26396447AB6DFD
2 changed files with 37 additions and 2 deletions

View File

@ -101,6 +101,9 @@ func (a ADV) SetTagBytes(t uint8, val []byte) (ok bool) {
return false
}
// delete the tag if it exists
a.DeleteTag(t)
// Header is in first 8 bytes.
i := 8
@ -116,13 +119,17 @@ func (a ADV) SetTagBytes(t uint8, val []byte) (ok bool) {
continue
}
// overflow check
if i+2+len(val) > AdvSize-4-2 {
return false
}
length := uint8(len(val))
a[i] = t
a[i+1] = length
copy(a[i+2:uint8(i+2)+length], val)
copy(a[i+2:i+2+int(length)], val)
ok = true

View File

@ -324,3 +324,31 @@ func TestADV_tail(t *testing.T) {
})
}
}
func TestADV_overwrite(t *testing.T) {
buf := make([]byte, 2*AdvSize)
a, err := NewADV(bytes.NewReader(buf))
if err != nil {
t.Errorf("NewADV() failed: %s", err)
}
for i := 0; i < 1024; i++ {
if !a.SetTag(adv.Bootonce, "yes") {
t.Errorf("SetTag() failed")
}
}
}
func TestADV_many_tags(t *testing.T) {
buf := make([]byte, 2*AdvSize)
a, err := NewADV(bytes.NewReader(buf))
if err != nil {
t.Errorf("NewADV() failed: %s", err)
}
for i := uint8(1); i < 255; i++ {
a.SetTag(i, "xa")
}
}