public inbox for libc-hacker@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Don't leak file descriptors in locale-archive reader
@ 2002-08-28 14:43 Jakub Jelinek
  2002-08-28 15:08 ` Ulrich Drepper
  0 siblings, 1 reply; 2+ messages in thread
From: Jakub Jelinek @ 2002-08-28 14:43 UTC (permalink / raw)
  To: Ulrich Drepper, Roland McGrath; +Cc: Glibc hackers

Hi!

As fd is not static, we are obviously leaking filedescriptors in lots of
places in _nl_load_locale_from_archive (and never close it even for the
successful return either).
I think that's the reason why /usr on separate partition doesn't shut down
cleanly.

2002-08-28  Jakub Jelinek  <jakub@redhat.com>

	* locale/loadarchive.c (_nl_load_locale_from_archive): Add fd >= 0
	check to close_and_out close.  Replace return NULL statements where
	fd might be >= 0 with goto close_and_out.  Close the file descriptor
	when it is no longer needed.

--- libc/locale/loadarchive.c.jj	2002-08-28 12:58:05.000000000 +0200
+++ libc/locale/loadarchive.c	2002-08-28 23:38:59.000000000 +0200
@@ -211,7 +211,8 @@ _nl_load_locale_from_archive (int catego
 	{
 	  /* stat failed, very strange.  */
 	close_and_out:
-	  __close (fd);
+	  if (fd >= 0)
+	    __close (fd);
 	  return NULL;
 	}
 
@@ -262,7 +263,7 @@ _nl_load_locale_from_archive (int catego
 
   /* If there is no archive or it cannot be loaded for some reason fail.  */
   if (__builtin_expect (headmap.ptr == NULL, 0))
-    return NULL;
+    goto close_and_out;
 
   /* We have the archive available.  To find the name we first have to
      determine its hash value.  */
@@ -281,7 +282,7 @@ _nl_load_locale_from_archive (int catego
     {
       if (namehashtab[idx].name_offset == 0)
 	/* Not found.  */
-	return NULL;
+	goto close_and_out;
 
       if (namehashtab[idx].hashval == hval
 	  && strcmp (name, headmap.ptr + namehashtab[idx].name_offset) == 0)
@@ -295,7 +296,7 @@ _nl_load_locale_from_archive (int catego
 
   /* We found an entry.  It might be a placeholder for a removed one.  */
   if (namehashtab[idx].locrec_offset == 0)
-    return NULL;
+    goto close_and_out;
 
   locrec = (struct locrecent *) (headmap.ptr + namehashtab[idx].locrec_offset);
 
@@ -309,7 +310,7 @@ _nl_load_locale_from_archive (int catego
 	    if (locrec->record[cnt].offset + locrec->record[cnt].len
 		> headmap.len)
 	      /* The archive locrectab contains bogus offsets.  */
-	      return NULL;
+	      goto close_and_out;
 	    results[cnt].addr = headmap.ptr + locrec->record[cnt].offset;
 	    results[cnt].len = locrec->record[cnt].len;
 	  }
@@ -376,7 +377,7 @@ _nl_load_locale_from_archive (int catego
 	      to = ranges[upper].from + ranges[upper].len;
 	      if (to > (size_t) archive_stat.st_size)
 		/* The archive locrectab contains bogus offsets.  */
-		return NULL;
+		goto close_and_out;
 	      to = (to + ps - 1) & ~(ps - 1);
 
 	      /* If a range is already mmaped in, stop.	 */
@@ -404,21 +405,21 @@ _nl_load_locale_from_archive (int catego
 		  || st.st_mtime != archive_stat.st_mtime
 		  || st.st_dev != archive_stat.st_dev
 		  || st.st_ino != archive_stat.st_ino)
-		return NULL;
+		goto close_and_out;
 	    }
 
 	  /* Map the range from the archive.  */
 	  addr = __mmap64 (NULL, to - from, PROT_READ, MAP_FILE|MAP_COPY,
 			   fd, from);
 	  if (addr == MAP_FAILED)
-	    return NULL;
+	    goto close_and_out;
 
 	  /* Allocate a record for this mapping.  */
 	  newp = (struct archmapped *) malloc (sizeof (struct archmapped));
 	  if (newp == NULL)
 	    {
 	      (void) __munmap (addr, to - from);
-	      return NULL;
+	      goto close_and_out;
 	    }
 
 	  /* And queue it.  */
@@ -443,6 +444,11 @@ _nl_load_locale_from_archive (int catego
 	}
     }
 
+  /* We don't need the file descriptor any longer.  */
+  if (fd >= 0)
+    __close (fd);
+  fd = -1;
+
   /* We succeeded in mapping all the necessary regions of the archive.
      Now we need the expected data structures to point into the data.  */
 

	Jakub

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] Don't leak file descriptors in locale-archive reader
  2002-08-28 14:43 [PATCH] Don't leak file descriptors in locale-archive reader Jakub Jelinek
@ 2002-08-28 15:08 ` Ulrich Drepper
  0 siblings, 0 replies; 2+ messages in thread
From: Ulrich Drepper @ 2002-08-28 15:08 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Roland McGrath, Glibc hackers

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Jakub Jelinek wrote:

> 2002-08-28  Jakub Jelinek  <jakub@redhat.com>
> 
> 	* locale/loadarchive.c (_nl_load_locale_from_archive): Add fd >= 0
> 	check to close_and_out close.  Replace return NULL statements where
> 	fd might be >= 0 with goto close_and_out.  Close the file descriptor
> 	when it is no longer needed.

Looks good, I've applied it.  Thanks,

- -- 
- ---------------.                          ,-.   1325 Chesapeake Terrace
Ulrich Drepper  \    ,-------------------'   \  Sunnyvale, CA 94089 USA
Red Hat          `--' drepper at redhat.com   `------------------------
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.0.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQE9bUn12ijCOnn/RHQRArPSAKCJURD0lCHs943Wn3cebx2WIWJ6bACgwTYG
dZcd46saLy2CVBy7trkdvNI=
=DoR4
-----END PGP SIGNATURE-----

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2002-08-28 22:08 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2002-08-28 14:43 [PATCH] Don't leak file descriptors in locale-archive reader Jakub Jelinek
2002-08-28 15:08 ` Ulrich Drepper

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).