Skip to content

Commit

Permalink
i2c(core): fix problems with usage of uintptr
Browse files Browse the repository at this point in the history
  • Loading branch information
@gen2thomas
gen2thomas committed Nov 11, 2023
1 parent a04ce8a commit 386de5d
Show file tree
Hide file tree
Showing 6 changed files with 97 additions and 40 deletions.
8 changes: 5 additions & 3 deletions drivers/i2c/i2c_connection_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,20 @@ import (

const dev = "/dev/i2c-1"

func getSyscallFuncImpl(errorMask byte) func(trap, a1, a2, a3 uintptr) (r1, r2 uintptr, err system.SyscallErrno) {
func getSyscallFuncImpl(
errorMask byte,
) func(trap, a1, a2 uintptr, a3 unsafe.Pointer) (r1, r2 uintptr, err system.SyscallErrno) {
// bit 0: error on function query
// bit 1: error on set address
// bit 2: error on command
return func(trap, a1, a2, a3 uintptr) (r1, r2 uintptr, err system.SyscallErrno) {
return func(trap, a1, a2 uintptr, a3 unsafe.Pointer) (r1, r2 uintptr, err system.SyscallErrno) {
// function query
if (trap == system.Syscall_SYS_IOCTL) && (a2 == system.I2C_FUNCS) {
if errorMask&0x01 == 0x01 {
return 0, 0, 1
}

var funcPtr *uint64 = (*uint64)(unsafe.Pointer(a3))
var funcPtr *uint64 = (*uint64)(a3)
*funcPtr = system.I2C_FUNC_SMBUS_READ_BYTE | system.I2C_FUNC_SMBUS_READ_BYTE_DATA |
system.I2C_FUNC_SMBUS_READ_WORD_DATA |
system.I2C_FUNC_SMBUS_WRITE_BYTE | system.I2C_FUNC_SMBUS_WRITE_BYTE_DATA |
Expand Down
27 changes: 18 additions & 9 deletions system/i2c_device.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,8 @@ func (d *i2cDevice) ReadBlockData(address int, reg uint8, data []byte) error {
buf := make([]byte, dataLen+1)
buf[0] = byte(dataLen)
copy(buf[1:], data)
if err := d.smbusAccess(address, I2C_SMBUS_READ, reg, I2C_SMBUS_I2C_BLOCK_DATA, unsafe.Pointer(&buf[0])); err != nil {
if err := d.smbusAccess(address, I2C_SMBUS_READ, reg, I2C_SMBUS_I2C_BLOCK_DATA,
unsafe.Pointer(&buf[0])); err != nil {
return err
}
// get data from buffer without first size element
Expand Down Expand Up @@ -322,7 +323,7 @@ func (d *i2cDevice) read(address int, b []byte) (n int, err error) {
func (d *i2cDevice) queryFunctionality(requested uint64, sender string) error {
// lazy initialization
if d.funcs == 0 {
if err := d.syscallIoctl(I2C_FUNCS, unsafe.Pointer(&d.funcs), "Querying functionality"); err != nil {
if err := d.syscallIoctl(I2C_FUNCS, unsafe.Pointer(&d.funcs), 0, "Querying functionality"); err != nil {
return err
}
}
Expand All @@ -334,7 +335,13 @@ func (d *i2cDevice) queryFunctionality(requested uint64, sender string) error {
return nil
}

func (d *i2cDevice) smbusAccess(address int, readWrite byte, command byte, protocol uint32, dataStart unsafe.Pointer) error {
func (d *i2cDevice) smbusAccess(
address int,
readWrite byte,
command byte,
protocol uint32,
dataStart unsafe.Pointer,
) error {
if err := d.setAddress(address); err != nil {
return err
}
Expand All @@ -346,8 +353,9 @@ func (d *i2cDevice) smbusAccess(address int, readWrite byte, command byte, proto
data: dataStart, // the reflected value of unsafePointer equals uintptr(dataStart),
}

sender := fmt.Sprintf("SMBus access r/w: %d, command: %d, protocol: %d, address: %d", readWrite, command, protocol, d.lastAddress)
if err := d.syscallIoctl(I2C_SMBUS, unsafe.Pointer(&smbus), sender); err != nil {
sender := fmt.Sprintf("SMBus access r/w: %d, command: %d, protocol: %d, address: %d",
readWrite, command, protocol, d.lastAddress)
if err := d.syscallIoctl(I2C_SMBUS, unsafe.Pointer(&smbus), 0, sender); err != nil {
return err
}

Expand All @@ -362,19 +370,20 @@ func (d *i2cDevice) setAddress(address int) error {
}
return nil
}
// for go vet false positives, see: https://github.com/golang/go/issues/41205
if err := d.syscallIoctl(I2C_SLAVE, unsafe.Pointer(uintptr(byte(address))), "Setting address"); err != nil {

// the payload is a pointer to a byte value, which contains the address
if err := d.syscallIoctl(I2C_SLAVE, nil, address, "Setting address"); err != nil {
return err
}
d.lastAddress = address
return nil
}

func (d *i2cDevice) syscallIoctl(signal uintptr, payload unsafe.Pointer, sender string) (err error) {
func (d *i2cDevice) syscallIoctl(signal uintptr, payload unsafe.Pointer, address int, sender string) (err error) {
if err := d.openFileLazy(sender); err != nil {
return err
}
if _, _, errno := d.sys.syscall(Syscall_SYS_IOCTL, d.file, signal, payload); errno != 0 {
if _, _, errno := d.sys.syscall(Syscall_SYS_IOCTL, d.file, signal, payload, uint16(address)); errno != 0 {
return fmt.Errorf("%s failed with syscall.Errno %v", sender, errno)
}

Expand Down
50 changes: 30 additions & 20 deletions system/i2c_device_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,20 @@ import (

const dev = "/dev/i2c-1"

func getSyscallFuncImpl(errorMask byte) func(trap, a1, a2, a3 uintptr) (r1, r2 uintptr, err SyscallErrno) {
func getSyscallFuncImpl(
errorMask byte,
) func(trap, a1, a2 uintptr, a3 unsafe.Pointer) (r1, r2 uintptr, err SyscallErrno) {
// bit 0: error on function query
// bit 1: error on set address
// bit 2: error on command
return func(trap, a1, a2, a3 uintptr) (r1, r2 uintptr, err SyscallErrno) {
return func(trap, a1, a2 uintptr, a3 unsafe.Pointer) (r1, r2 uintptr, err SyscallErrno) {
// function query
if (trap == Syscall_SYS_IOCTL) && (a2 == I2C_FUNCS) {
if errorMask&0x01 == 0x01 {
return 0, 0, 1
}

var funcPtr *uint64 = (*uint64)(unsafe.Pointer(a3))
var funcPtr *uint64 = (*uint64)(a3)
*funcPtr = I2C_FUNC_SMBUS_READ_BYTE | I2C_FUNC_SMBUS_READ_BYTE_DATA |
I2C_FUNC_SMBUS_READ_WORD_DATA |
I2C_FUNC_SMBUS_WRITE_BYTE | I2C_FUNC_SMBUS_WRITE_BYTE_DATA |
Expand Down Expand Up @@ -115,7 +117,7 @@ func TestWriteRead(t *testing.T) {
func TestReadByte(t *testing.T) {
tests := map[string]struct {
funcs uint64
syscallImpl func(trap, a1, a2, a3 uintptr) (r1, r2 uintptr, err SyscallErrno)
syscallImpl func(trap, a1, a2 uintptr, a3 unsafe.Pointer) (r1, r2 uintptr, err SyscallErrno)
wantErr string
}{
"read_byte_ok": {
Expand All @@ -124,7 +126,8 @@ func TestReadByte(t *testing.T) {
"error_syscall": {
funcs: I2C_FUNC_SMBUS_READ_BYTE,
syscallImpl: getSyscallFuncImpl(0x04),
wantErr: "SMBus access r/w: 1, command: 0, protocol: 1, address: 2 failed with syscall.Errno operation not permitted",
wantErr: "SMBus access r/w: 1, command: 0, protocol: 1, address: 2 " +
"failed with syscall.Errno operation not permitted",
},
"error_not_supported": {
wantErr: "SMBus read byte not supported",
Expand Down Expand Up @@ -159,7 +162,7 @@ func TestReadByte(t *testing.T) {
func TestReadByteData(t *testing.T) {
tests := map[string]struct {
funcs uint64
syscallImpl func(trap, a1, a2, a3 uintptr) (r1, r2 uintptr, err SyscallErrno)
syscallImpl func(trap, a1, a2 uintptr, a3 unsafe.Pointer) (r1, r2 uintptr, err SyscallErrno)
wantErr string
}{
"read_byte_data_ok": {
Expand All @@ -168,7 +171,8 @@ func TestReadByteData(t *testing.T) {
"error_syscall": {
funcs: I2C_FUNC_SMBUS_READ_BYTE_DATA,
syscallImpl: getSyscallFuncImpl(0x04),
wantErr: "SMBus access r/w: 1, command: 1, protocol: 2, address: 3 failed with syscall.Errno operation not permitted",
wantErr: "SMBus access r/w: 1, command: 1, protocol: 2, address: 3 " +
"failed with syscall.Errno operation not permitted",
},
"error_not_supported": {
wantErr: "SMBus read byte data not supported",
Expand Down Expand Up @@ -206,7 +210,7 @@ func TestReadByteData(t *testing.T) {
func TestReadWordData(t *testing.T) {
tests := map[string]struct {
funcs uint64
syscallImpl func(trap, a1, a2, a3 uintptr) (r1, r2 uintptr, err SyscallErrno)
syscallImpl func(trap, a1, a2 uintptr, a3 unsafe.Pointer) (r1, r2 uintptr, err SyscallErrno)
wantErr string
}{
"read_word_data_ok": {
Expand All @@ -215,7 +219,8 @@ func TestReadWordData(t *testing.T) {
"error_syscall": {
funcs: I2C_FUNC_SMBUS_READ_WORD_DATA,
syscallImpl: getSyscallFuncImpl(0x04),
wantErr: "SMBus access r/w: 1, command: 2, protocol: 3, address: 4 failed with syscall.Errno operation not permitted",
wantErr: "SMBus access r/w: 1, command: 2, protocol: 3, address: 4 " +
"failed with syscall.Errno operation not permitted",
},
"error_not_supported": {
wantErr: "SMBus read word data not supported",
Expand Down Expand Up @@ -270,7 +275,7 @@ func TestReadBlockData(t *testing.T) {
)
tests := map[string]struct {
funcs uint64
syscallImpl func(trap, a1, a2, a3 uintptr) (r1, r2 uintptr, err SyscallErrno)
syscallImpl func(trap, a1, a2 uintptr, a3 unsafe.Pointer) (r1, r2 uintptr, err SyscallErrno)
wantErr string
}{
"read_block_data_ok": {
Expand All @@ -279,7 +284,8 @@ func TestReadBlockData(t *testing.T) {
"error_syscall": {
funcs: I2C_FUNC_SMBUS_READ_I2C_BLOCK,
syscallImpl: getSyscallFuncImpl(0x04),
wantErr: "SMBus access r/w: 1, command: 3, protocol: 8, address: 5 failed with syscall.Errno operation not permitted",
wantErr: "SMBus access r/w: 1, command: 3, protocol: 8, address: 5 " +
"failed with syscall.Errno operation not permitted",
},
"error_from_used_fallback_if_not_supported": {
wantErr: "Read 1 bytes from device by sysfs, expected 10",
Expand Down Expand Up @@ -315,7 +321,7 @@ func TestReadBlockData(t *testing.T) {
func TestWriteByte(t *testing.T) {
tests := map[string]struct {
funcs uint64
syscallImpl func(trap, a1, a2, a3 uintptr) (r1, r2 uintptr, err SyscallErrno)
syscallImpl func(trap, a1, a2 uintptr, a3 unsafe.Pointer) (r1, r2 uintptr, err SyscallErrno)
wantErr string
}{
"write_byte_ok": {
Expand All @@ -324,7 +330,8 @@ func TestWriteByte(t *testing.T) {
"error_syscall": {
funcs: I2C_FUNC_SMBUS_WRITE_BYTE,
syscallImpl: getSyscallFuncImpl(0x04),
wantErr: "SMBus access r/w: 0, command: 68, protocol: 1, address: 6 failed with syscall.Errno operation not permitted",
wantErr: "SMBus access r/w: 0, command: 68, protocol: 1, address: 6 " +
"failed with syscall.Errno operation not permitted",
},
"error_not_supported": {
wantErr: "SMBus write byte not supported",
Expand Down Expand Up @@ -357,7 +364,7 @@ func TestWriteByte(t *testing.T) {
func TestWriteByteData(t *testing.T) {
tests := map[string]struct {
funcs uint64
syscallImpl func(trap, a1, a2, a3 uintptr) (r1, r2 uintptr, err SyscallErrno)
syscallImpl func(trap, a1, a2 uintptr, a3 unsafe.Pointer) (r1, r2 uintptr, err SyscallErrno)
wantErr string
}{
"write_byte_data_ok": {
Expand All @@ -366,7 +373,8 @@ func TestWriteByteData(t *testing.T) {
"error_syscall": {
funcs: I2C_FUNC_SMBUS_WRITE_BYTE_DATA,
syscallImpl: getSyscallFuncImpl(0x04),
wantErr: "SMBus access r/w: 0, command: 4, protocol: 2, address: 7 failed with syscall.Errno operation not permitted",
wantErr: "SMBus access r/w: 0, command: 4, protocol: 2, address: 7 " +
"failed with syscall.Errno operation not permitted",
},
"error_not_supported": {
wantErr: "SMBus write byte data not supported",
Expand Down Expand Up @@ -404,7 +412,7 @@ func TestWriteByteData(t *testing.T) {
func TestWriteWordData(t *testing.T) {
tests := map[string]struct {
funcs uint64
syscallImpl func(trap, a1, a2, a3 uintptr) (r1, r2 uintptr, err SyscallErrno)
syscallImpl func(trap, a1, a2 uintptr, a3 unsafe.Pointer) (r1, r2 uintptr, err SyscallErrno)
wantErr string
}{
"write_word_data_ok": {
Expand All @@ -413,7 +421,8 @@ func TestWriteWordData(t *testing.T) {
"error_syscall": {
funcs: I2C_FUNC_SMBUS_WRITE_WORD_DATA,
syscallImpl: getSyscallFuncImpl(0x04),
wantErr: "SMBus access r/w: 0, command: 5, protocol: 3, address: 8 failed with syscall.Errno operation not permitted",
wantErr: "SMBus access r/w: 0, command: 5, protocol: 3, address: 8 " +
"failed with syscall.Errno operation not permitted",
},
"error_not_supported": {
wantErr: "SMBus write word data not supported",
Expand Down Expand Up @@ -469,7 +478,7 @@ func TestWriteBlockData(t *testing.T) {
)
tests := map[string]struct {
funcs uint64
syscallImpl func(trap, a1, a2, a3 uintptr) (r1, r2 uintptr, err SyscallErrno)
syscallImpl func(trap, a1, a2 uintptr, a3 unsafe.Pointer) (r1, r2 uintptr, err SyscallErrno)
wantErr string
}{
"write_block_data_ok": {
Expand All @@ -478,7 +487,8 @@ func TestWriteBlockData(t *testing.T) {
"error_syscall": {
funcs: I2C_FUNC_SMBUS_WRITE_I2C_BLOCK,
syscallImpl: getSyscallFuncImpl(0x04),
wantErr: "SMBus access r/w: 0, command: 6, protocol: 8, address: 9 failed with syscall.Errno operation not permitted",
wantErr: "SMBus access r/w: 0, command: 6, protocol: 8, address: 9 " +
"failed with syscall.Errno operation not permitted",
},
}
for name, tc := range tests {
Expand Down Expand Up @@ -531,7 +541,7 @@ func Test_queryFunctionality(t *testing.T) {
tests := map[string]struct {
requested uint64
dev string
syscallImpl func(trap, a1, a2, a3 uintptr) (r1, r2 uintptr, err SyscallErrno)
syscallImpl func(trap, a1, a2 uintptr, a3 unsafe.Pointer) (r1, r2 uintptr, err SyscallErrno)
wantErr string
wantFile bool
wantFuncs uint64
Expand Down
26 changes: 24 additions & 2 deletions system/syscall.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,30 @@ const (
type nativeSyscall struct{}

// Syscall calls the native unix.Syscall, implements the SystemCaller interface
func (sys *nativeSyscall) syscall(trap uintptr, f File, signal uintptr, payload unsafe.Pointer) (r1, r2 uintptr, err SyscallErrno) {
r1, r2, errNo := unix.Syscall(trap, f.Fd(), signal, uintptr(payload))
// Note: It would be possible to transfer the address as an unsafe.Pointer to e.g. a byte, uint16 or integer variable.
// The unpack process here would be as follows:
// * convert the payload back to the pointer: addrPtr := (*byte)(payload)
// * call with the content converted to uintptr: r1, r2, errNo = unix.Syscall(trap, f.Fd(), signal, uintptr(*addrPtr))
// This has the main disadvantage, that if someone change the type of the address at caller side, the compiler will not
// detect this problem and this unpack procedure would cause unpredictable results.
// So the decision was taken to give the address here as a separate parameter, although it is not used in every call.
// Note also, that the size of the address variable at Kernel side is u16, therefore uint16 is used here.
func (sys *nativeSyscall) syscall(
trap uintptr,
f File,
signal uintptr,
payload unsafe.Pointer,
address uint16,
) (r1, r2 uintptr, err SyscallErrno) {
var errNo unix.Errno
if signal == I2C_SLAVE {
// this is the setup for the address, it just needs to be converted to an uintptr,
// the given payload is not used in this case, see the comment on the function
r1, r2, errNo = unix.Syscall(trap, f.Fd(), signal, uintptr(address))
} else {
r1, r2, errNo = unix.Syscall(trap, f.Fd(), signal, uintptr(payload))
}

return r1, r2, SyscallErrno(errNo)
}

Expand Down
17 changes: 12 additions & 5 deletions system/syscall_mock.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,25 @@ type mockSyscall struct {
smbus *i2cSmbusIoctlData
sliceSize uint8
dataSlice []byte
Impl func(trap, a1, a2, a3 uintptr) (r1, r2 uintptr, err SyscallErrno)
Impl func(trap, a1, a2 uintptr, a3 unsafe.Pointer) (r1, r2 uintptr, err SyscallErrno)
}

// Syscall calls the user defined implementation, used for tests, implements the SystemCaller interface
func (sys *mockSyscall) syscall(trap uintptr, f File, signal uintptr, payload unsafe.Pointer) (r1, r2 uintptr, err SyscallErrno) {
func (sys *mockSyscall) syscall(
trap uintptr,
f File,
signal uintptr,
payload unsafe.Pointer,
address uint16,
) (r1, r2 uintptr, err SyscallErrno) {
sys.lastTrap = trap // points to the used syscall (e.g. "SYS_IOCTL")
sys.lastFile = f // a character device file (e.g. file to path "/dev/i2c-1")
sys.lastSignal = signal // points to used function type (e.g. I2C_SMBUS, I2C_RDWR)

if signal == I2C_SLAVE {
// in this case the uintptr corresponds the address
sys.devAddress = uintptr(payload)
// this is the setup for the address, it needs to be converted to an uintptr,
// the given payload is not used in this case, see the comment on the function used for production
sys.devAddress = uintptr(address)
}

if signal == I2C_SMBUS {
Expand Down Expand Up @@ -54,7 +61,7 @@ func (sys *mockSyscall) syscall(trap uintptr, f File, signal uintptr, payload un

// call mock implementation
if sys.Impl != nil {
return sys.Impl(trap, f.Fd(), signal, uintptr(payload))
return sys.Impl(trap, f.Fd(), signal, payload)
}
return 0, 0, 0
}
Expand Down
9 changes: 8 additions & 1 deletion system/system.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,15 @@ type filesystem interface {

// systemCaller represents unexposed Syscall interface to allow the switch between native and mocked implementation
// Prevent unsafe call, since go 1.15, see "Pattern 4" in: https://go101.org/article/unsafe.html
// For go vet false positives, see: https://github.com/golang/go/issues/41205
type systemCaller interface {
syscall(trap uintptr, f File, signal uintptr, payload unsafe.Pointer) (r1, r2 uintptr, err SyscallErrno)
syscall(
trap uintptr,
f File,
signal uintptr,
payload unsafe.Pointer,
address uint16,
) (r1, r2 uintptr, err SyscallErrno)
}

// digitalPinAccesser represents unexposed interface to allow the switch between different implementations and
Expand Down

0 comments on commit 386de5d

Please sign in to comment.