| Message ID | 20250513173539.39952965@endymion |
|---|---|
| State | New |
| Headers | show |
| Series | [1/3] i2ctransfer: Don't free memory which was never allocated | expand |
On Tue, May 13, 2025 at 05:35:39PM +0200, Jean Delvare wrote: > Valgrind complains that uninitialized memory may be passed to > ioctl(): > > == Syscall param ioctl(I2C_RDWR) points to uninitialised byte(s) > == at 0x499382B: ioctl (in /lib64/libc.so.6) > == by 0x401957: main (i2ctransfer.c:343) > == Address 0x1ffefff94c is on thread 1's stack > == in frame #1, created by main (i2ctransfer.c:144) > == > == Syscall param ioctl(I2C_RDWR).msgs points to uninitialised byte(s) > == at 0x499382B: ioctl (in /lib64/libc.so.6) > == by 0x401957: main (i2ctransfer.c:343) > == Address 0x1ffefff956 is on thread 1's stack > == in frame #1, created by main (i2ctransfer.c:144) > > Zero out the i2c_rdwr_ioctl_data struct as well as the msgs array to > guarantee that no uninitialized memory will ever be passed to the > kernel. > > Signed-off-by: Jean Delvare <jdelvare@suse.de> Applied, thanks! > This one is not strictly needed, I can't see any actual bug. However > making valgrind happy seems to be a sane goal, so that we can keep > using it when debugging other issues without getting distracted. > Wolfram, what do you think? Sure, I agree. Also, I like the simpler code.
--- i2c-tools.orig/tools/i2ctransfer.c +++ i2c-tools/tools/i2ctransfer.c @@ -149,8 +149,7 @@ int main(int argc, char *argv[]) enum parse_state state = PARSE_GET_DESC; unsigned int buf_idx = 0; - for (i = 0; i < I2C_RDRW_IOCTL_MAX_MSGS; i++) - msgs[i].buf = NULL; + memset(msgs, 0, sizeof(msgs)); /* handle (optional) flags first */ while ((opt = getopt(argc, argv, "abfhvVy")) != -1) { @@ -334,6 +333,7 @@ int main(int argc, char *argv[]) struct i2c_rdwr_ioctl_data rdwr; unsigned int print_flags = PRINT_READ_BUF; + memset(&rdwr, 0, sizeof(rdwr)); rdwr.msgs = msgs; rdwr.nmsgs = nmsgs; nmsgs_sent = ioctl(file, I2C_RDWR, &rdwr);
Valgrind complains that uninitialized memory may be passed to ioctl(): == Syscall param ioctl(I2C_RDWR) points to uninitialised byte(s) == at 0x499382B: ioctl (in /lib64/libc.so.6) == by 0x401957: main (i2ctransfer.c:343) == Address 0x1ffefff94c is on thread 1's stack == in frame #1, created by main (i2ctransfer.c:144) == == Syscall param ioctl(I2C_RDWR).msgs points to uninitialised byte(s) == at 0x499382B: ioctl (in /lib64/libc.so.6) == by 0x401957: main (i2ctransfer.c:343) == Address 0x1ffefff956 is on thread 1's stack == in frame #1, created by main (i2ctransfer.c:144) Zero out the i2c_rdwr_ioctl_data struct as well as the msgs array to guarantee that no uninitialized memory will ever be passed to the kernel. Signed-off-by: Jean Delvare <jdelvare@suse.de> --- This one is not strictly needed, I can't see any actual bug. However making valgrind happy seems to be a sane goal, so that we can keep using it when debugging other issues without getting distracted. Wolfram, what do you think? tools/i2ctransfer.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)