public inbox for libc-hacker@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Fix localedef
@ 2009-04-27  9:12 Jakub Jelinek
  0 siblings, 0 replies; 2+ messages in thread
From: Jakub Jelinek @ 2009-04-27  9:12 UTC (permalink / raw)
  To: Ulrich Drepper; +Cc: Glibc hackers

Hi!

We still have a bunch of issues in localedef:
1) MAP_ANON without MAP_PRIVATE or MAP_SHARED is MAP_FAILED/EINVAL,
   so we actually never reserved anything
2) enlarge_archive, if the reserved region size isn't big enough,
   now munmaps the old region and mmaps the whole file elsewhere, but
   doesn't update the head pointer, which now points to the munmaped
   area
3) unfortunately, it seems kernel rejects mremap MREMAP_MAYMOVE|MREMAP_FIXED
   if new_addr == addr (-1/EINVAL):
		/* Check if the location we're moving into overlaps the
		 * old location at all, and fail if it does.
		 */
		if ((new_addr <= addr) && (new_addr+new_len) > addr)
			goto out;

		if ((addr <= new_addr) && (addr+old_len) > new_addr)
			goto out;
This patch fixes 1) by oring in MAP_PRIVATE, 2) by head = ah->addr,
3) by replacing mremap with mmap64 (for speed I'm just mmaping starting
at end of previous mmapping rounded down to page boundary) and in addition
to this it adds the RESERVE_MMAP_SIZE stuff to enlarge_archive, as e.g.
when --add-to-archive hundreds of locales into an empty locale-archive
this makes quite a significant difference.

2009-04-27  Jakub Jelinek  <jakub@redhat.com>

	* locale/programs/locarchive.c (create_archive): Add MAP_PRIVATE
	to MAP_ANON in PROT_NONE mmap64 call.
	(open_archive): Likewise.
	(file_data_available_p): Use mmap64 instead of mremap.
	(enlarge_archive): Likewise.  Update head if ah->addr changed.
	Attempt to reserve address space after mmap64 region.

--- libc/locale/programs/locarchive.c.jj	2009-04-27 10:44:42.000000000 +0200
+++ libc/locale/programs/locarchive.c	2009-04-27 10:45:42.000000000 +0200
@@ -134,8 +134,8 @@ create_archive (const char *archivefname
   size_t reserved = RESERVE_MMAP_SIZE;
   int xflags = 0;
   if (total < reserved
-      && ((p = mmap64 (NULL, reserved, PROT_NONE, MAP_ANON, -1, 0))
-	  != MAP_FAILED))
+      && ((p = mmap64 (NULL, reserved, PROT_NONE, MAP_PRIVATE | MAP_ANON,
+		       -1, 0)) != MAP_FAILED))
     xflags = MAP_FIXED;
   else
     {
@@ -259,10 +259,16 @@ file_data_available_p (struct locarhandl
   if (st.st_size > ah->reserved)
     return false;
 
-  void *p = mremap (ah->addr, ah->mmaped, st.st_size,
-		    MREMAP_FIXED | MREMAP_MAYMOVE, ah->addr);
+  const size_t pagesz = getpagesize ();
+  size_t start = ah->mmaped & ~(pagesz - 1);
+  void *p = mmap64 (ah->addr + start, st.st_size - start,
+		    PROT_READ | PROT_WRITE, MAP_SHARED | MAP_FIXED,
+		    ah->fd, start);
   if (p == MAP_FAILED)
-    return false;
+    {
+      ah->mmaped = start;
+      return false;
+    }
 
   ah->mmaped = st.st_size;
   return true;
@@ -312,14 +318,15 @@ enlarge_archive (struct locarhandle *ah,
     error (EXIT_FAILURE, errno, _("cannot map locale archive file"));
 
   if (st.st_size < ah->reserved)
-    ah->addr = mremap (ah->addr, ah->mmaped, st.st_size,
-		       MREMAP_MAYMOVE | MREMAP_FIXED, ah->addr);
+    ah->addr = mmap64 (ah->addr, st.st_size, PROT_READ | PROT_WRITE,
+		       MAP_SHARED | MAP_FIXED, ah->fd, 0);
   else
     {
       munmap (ah->addr, ah->reserved);
       ah->addr = mmap64 (NULL, st.st_size, PROT_READ | PROT_WRITE,
 			 MAP_SHARED, ah->fd, 0);
       ah->reserved = st.st_size;
+      head = ah->addr;
     }
   if (ah->addr == MAP_FAILED)
     goto enomap;
@@ -384,8 +391,22 @@ enlarge_archive (struct locarhandle *ah,
       error (EXIT_FAILURE, errval, _("cannot resize archive file"));
     }
 
+  /* To prepare for enlargements of the mmaped area reserve some
+     address space.  */
+  size_t reserved = RESERVE_MMAP_SIZE;
+  int xflags = 0;
+  if (total < reserved
+      && ((p = mmap64 (NULL, reserved, PROT_NONE, MAP_PRIVATE | MAP_ANON,
+		       -1, 0)) != MAP_FAILED))
+    xflags = MAP_FIXED;
+  else
+    {
+      p = NULL;
+      reserved = total;
+    }
+
   /* Map the header and all the administration data structures.  */
-  p = mmap64 (NULL, total, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
+  p = mmap64 (p, total, PROT_READ | PROT_WRITE, MAP_SHARED | xflags, fd, 0);
   if (p == MAP_FAILED)
     {
       int errval = errno;
@@ -404,7 +425,7 @@ enlarge_archive (struct locarhandle *ah,
   new_ah.mmaped = total;
   new_ah.addr = p;
   new_ah.fd = fd;
-  new_ah.reserved = total;
+  new_ah.reserved = reserved;
 
   /* Walk through the hash name hash table to find out what data is
      still referenced and transfer it into the new file.  */
@@ -593,8 +614,8 @@ open_archive (struct locarhandle *ah, bo
   int xflags = 0;
   void *p;
   if (st.st_size < reserved
-      && ((p = mmap64 (NULL, RESERVE_MMAP_SIZE, PROT_NONE, MAP_ANON, -1, 0))
-	  != MAP_FAILED))
+      && ((p = mmap64 (NULL, reserved, PROT_NONE, MAP_PRIVATE | MAP_ANON,
+		       -1, 0)) != MAP_FAILED))
     xflags = MAP_FIXED;
   else
     {

	Jakub

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

* [PATCH] Fix localedef
@ 2002-10-22  7:00 Jakub Jelinek
  0 siblings, 0 replies; 2+ messages in thread
From: Jakub Jelinek @ 2002-10-22  7:00 UTC (permalink / raw)
  To: Roland McGrath, Ulrich Drepper; +Cc: Glibc hackers

Hi!

localedef can create bogus archive if enlarge_archive happens in add_alias
and not in add_locale. The problem is that add_locale_to_archive keeps the
old locrec_offset accross archive enlargement.  Another problem is
that add_alias needs to be given exactly the same name as has been passed
to add_locale initially, while it passed name unconditionally in the first
case and already free'd memory in the second case.

2002-10-22  Jakub Jelinek  <jakub@redhat.com>

	* locale/programs/locarchive.c (add_alias): Change locrec_offset arg
	into pointer to locrec_offset.
	(add_locale_to_archive): Adjust callers.  Free normalized_name right
	before returning, not immediately after add_locale, pass it to
	add_alias if not NULL instead of name.  Rename second normalized_name
	occurence to nnormalized_codeset_name.

--- libc/locale/programs/locarchive.c.jj	2002-10-22 15:34:50.000000000 +0200
+++ libc/locale/programs/locarchive.c	2002-10-22 15:34:37.000000000 +0200
@@ -583,8 +583,9 @@ insert_name (struct locarhandle *ah,
 
 static void
 add_alias (struct locarhandle *ah, const char *alias, bool replace,
-	   const char *oldname, uint32_t locrec_offset)
+	   const char *oldname, uint32_t *locrec_offset_p)
 {
+  uint32_t locrec_offset = *locrec_offset_p;
   struct locarhead *head = ah->addr;
   const size_t name_len = strlen (alias);
   struct namehashent *namehashent = insert_name (ah, alias, strlen (alias),
@@ -607,10 +608,10 @@ add_alias (struct locarhandle *ah, const
 	  namehashent = insert_name (ah, oldname, strlen (oldname), true);
 	  assert (namehashent->name_offset != 0);
 	  assert (namehashent->locrec_offset != 0);
-	  locrec_offset = namehashent->locrec_offset;
+	  *locrec_offset_p = namehashent->locrec_offset;
 
 	  /* Tail call to try the whole thing again.  */
-	  add_alias (ah, alias, replace, oldname, locrec_offset);
+	  add_alias (ah, alias, replace, oldname, locrec_offset_p);
 	  return;
 	}
 
@@ -932,9 +933,9 @@ add_locale_to_archive (ah, name, data, r
 
   /* This call does the main work.  */
   locrec_offset = add_locale (ah, normalized_name ?: name, data, replace);
-  free (normalized_name);
   if (locrec_offset == 0)
     {
+      free (normalized_name);
       if (mask & XPG_NORM_CODESET)
 	free ((char *) normalized_codeset);
       return -1;
@@ -953,17 +954,19 @@ add_locale_to_archive (ah, name, data, r
       } *filedata = data[LC_CTYPE].addr;
       codeset = (char *) filedata
 	+ filedata->strindex[_NL_ITEM_INDEX (_NL_CTYPE_CODESET_NAME)];
+      char *normalized_codeset_name = NULL;
 
       normalized_codeset = _nl_normalize_codeset (codeset, strlen (codeset));
       mask |= XPG_NORM_CODESET;
 
-      asprintf (&normalized_name, "%s%s%s.%s%s%s",
+      asprintf (&normalized_codeset_name, "%s%s%s.%s%s%s",
 		language, territory == NULL ? "" : "_", territory ?: "",
 		normalized_codeset,
 		modifier == NULL ? "" : "@", modifier ?: "");
 
-      add_alias (ah, normalized_name, replace, name, locrec_offset);
-      free (normalized_name);
+      add_alias (ah, normalized_codeset_name, replace,
+		 normalized_name ?: name, &locrec_offset);
+      free (normalized_codeset_name);
     }
 
   /* Now read the locale.alias files looking for lines whose
@@ -1061,7 +1064,7 @@ add_locale_to_archive (ah, name, data, r
 			&& !strcmp (modifier ?: "", rhs_modifier ?: ""))
 		      /* We have a winner.  */
 		      add_alias (ah, alias, replace,
-				 normalized_name ?: name, locrec_offset);
+				 normalized_name ?: name, &locrec_offset);
 		    if (rhs_mask & XPG_NORM_CODESET)
 		      free ((char *) rhs_normalized_codeset);
 		  }
@@ -1083,6 +1086,8 @@ add_locale_to_archive (ah, name, data, r
       fclose (fp);
     }
 
+  free (normalized_name);
+
   if (mask & XPG_NORM_CODESET)
     free ((char *) normalized_codeset);
 

	Jakub

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

end of thread, other threads:[~2009-04-27  9:12 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-04-27  9:12 [PATCH] Fix localedef Jakub Jelinek
  -- strict thread matches above, loose matches on Subject: below --
2002-10-22  7:00 Jakub Jelinek

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).