Message ID | 20250513172119.09548573@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:21:19PM +0200, Jean Delvare wrote: > If an error occurs while msgs[] is been prepared for the transfer, > we jump to the clean-up path. How many buffers need to be freed > depends on the state. If we were parsing data, we should free up to > nmsgs. However, if we were parsing descriptors, we should free > up to nmsgs - 1 only. The code was unconditionally freeing up to > nmsgs, potentially freeing a non-allocated buffer. > > In most cases, it was not a problem, we would simply call free() on a > NULL pointer and that's a no-op. However, if msgs[] was full then we > would access memory beyond its end and call free() on a random > pointer. > > Fixes: 9fc53a7fc669 ("i2c-tools: add new tool 'i2ctransfer'") > Signed-off-by: Jean Delvare <jdelvare@suse.de> Applied, thanks!
--- i2c-tools.orig/tools/i2ctransfer.c +++ i2c-tools/tools/i2ctransfer.c @@ -364,7 +364,13 @@ int main(int argc, char *argv[]) err_out: close(file); - for (i = 0; i <= nmsgs; i++) + /* + * If we were parsing data, the buffer for the last message was + * already allocated and nmsgs still points to it. + */ + if (state == PARSE_GET_DATA) + free(msgs[nmsgs].buf); + for (i = 0; i < nmsgs; i++) free(msgs[i].buf); exit(1);
If an error occurs while msgs[] is been prepared for the transfer, we jump to the clean-up path. How many buffers need to be freed depends on the state. If we were parsing data, we should free up to nmsgs. However, if we were parsing descriptors, we should free up to nmsgs - 1 only. The code was unconditionally freeing up to nmsgs, potentially freeing a non-allocated buffer. In most cases, it was not a problem, we would simply call free() on a NULL pointer and that's a no-op. However, if msgs[] was full then we would access memory beyond its end and call free() on a random pointer. Fixes: 9fc53a7fc669 ("i2c-tools: add new tool 'i2ctransfer'") Signed-off-by: Jean Delvare <jdelvare@suse.de> --- tools/i2ctransfer.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)