Message ID | 20250516152538.75325-1-ant.v.moryakov@gmail.com |
---|---|
State | Accepted |
Delegated to: | Tom Rini |
Headers | show |
Series | tools: fix handle leak in ifdtool.c | expand |
Hi Anton, On 5/16/25 5:25 PM, ant.v.moryakov@gmail.com wrote: > From: Anton Moryakov <ant.v.moryakov@gmail.com> > > Prevent file descriptor leaks by properly closing 'fd' and 'new_fd' > when fstat() or write() operations fail. > > - Added close(fd) before return in open_for_read() if fstat() fails. > - Added close(new_fd) before return in write_image() if write() fails. > - No close needed if open() fails (fd == -1 is invalid). > > Signed-off-by: Anton Moryakov <ant.v.moryakov@gmail.com> Looks fine to me Reviewed-by: Quentin Schulz <quentin.schulz@cherry.de> I believe there are a few others in that file? inject_region returns without closing, same for write_data, same for the main function with bios_fd at the very least. Thanks! Quentin
Maybe. However, the static analyser did not swear at others Пн, 19 мая 2025 г. в 11:47, Quentin Schulz <quentin.schulz@cherry.de>: > Hi Anton, > > On 5/16/25 5:25 PM, ant.v.moryakov@gmail.com wrote: > > From: Anton Moryakov <ant.v.moryakov@gmail.com> > > > > Prevent file descriptor leaks by properly closing 'fd' and 'new_fd' > > when fstat() or write() operations fail. > > > > - Added close(fd) before return in open_for_read() if fstat() fails. > > - Added close(new_fd) before return in write_image() if write() fails. > > - No close needed if open() fails (fd == -1 is invalid). > > > > Signed-off-by: Anton Moryakov <ant.v.moryakov@gmail.com> > > Looks fine to me > > Reviewed-by: Quentin Schulz <quentin.schulz@cherry.de> > > I believe there are a few others in that file? > > inject_region returns without closing, same for write_data, same for the > main function with bios_fd at the very least. > > Thanks! > Quentin >
On Fri, 16 May 2025 18:25:38 +0300, ant.v.moryakov@gmail.com wrote: > Prevent file descriptor leaks by properly closing 'fd' and 'new_fd' > when fstat() or write() operations fail. > > - Added close(fd) before return in open_for_read() if fstat() fails. > - Added close(new_fd) before return in write_image() if write() fails. > - No close needed if open() fails (fd == -1 is invalid). > > [...] Applied to u-boot/next, thanks! [1/1] tools: fix handle leak in ifdtool.c commit: a4fb99ce4e6ee62f3278d7472b9b9282d7b289ed
diff --git a/tools/ifdtool.c b/tools/ifdtool.c index b70570361f4..5db6fa7d195 100644 --- a/tools/ifdtool.c +++ b/tools/ifdtool.c @@ -499,8 +499,10 @@ static int write_image(char *filename, char *image, int size) S_IWUSR | S_IRGRP | S_IROTH); if (new_fd < 0) return perror_fname("Could not open file '%s'", filename); - if (write(new_fd, image, size) != size) + if (write(new_fd, image, size) != size) { + close(new_fd); return perror_fname("Could not write file '%s'", filename); + } close(new_fd); return 0; @@ -604,8 +606,10 @@ int open_for_read(const char *fname, int *sizep) if (fd == -1) return perror_fname("Could not open file '%s'", fname); - if (fstat(fd, &buf) == -1) + if (fstat(fd, &buf) == -1) { + close(fd); return perror_fname("Could not stat file '%s'", fname); + } *sizep = buf.st_size; debug("File %s is %d bytes\n", fname, *sizep);