On 10/11/18 12:13 PM, Walter Harms wrote:


Alan Coopersmith <alan.coopersm...@oracle.com> hat am 1. Oktober 2018 um 00:14
geschrieben:


Found by Oracle's Parfait 2.2 static analyzer:

Error: File Leak
    File Leak [file-ptr-leak]:
       Leaked File fp
         at line 94 of lib/libXpm/src/RdFToBuf.c in function
'XpmReadFileToBuffer
'.
           fp initialized at line 86 with fdopen
           fp leaks when len < 0 at line 92.

Introduced-by: commit 8b3024e6871ce50b34bf2dff924774bd654703bc

Signed-off-by: Alan Coopersmith <alan.coopersm...@oracle.com>
---
  src/RdFToBuf.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/RdFToBuf.c b/src/RdFToBuf.c
index 69e3347..1b386f8 100644
--- a/src/RdFToBuf.c
+++ b/src/RdFToBuf.c
@@ -86,15 +86,15 @@ XpmReadFileToBuffer(
      fp = fdopen(fd, "r");
      if (!fp) {
        close(fd);
        return XpmOpenFailed;
      }
      len = stats.st_size;
      if (len < 0 || len >= SIZE_MAX) {
-       close(fd);
+       fclose(fp);
        return XpmOpenFailed;
      }

IMHO it should do both,
otherwise you have a different behavier when
fdopen failed and returning XpmOpenFailed
or size check failed and returning XpmOpenFailed

Once fp = fdopen(fd, ...) succeeds, then fclose(fp) will also close the
underlying fd, so us trying to close it again will cause EBADF errors in
most cases, or possible race conditions if libXpm is being used in a
multi-threaded program that opens another fd in another thread at the
same time.

--
        -Alan Coopersmith-               alan.coopersm...@oracle.com
         Oracle Solaris Engineering - https://blogs.oracle.com/alanc
_______________________________________________
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Reply via email to