[parted-devel] [PATCH v4] dos: improve MBR signature generation

Jim Meyering jim at meyering.net
Thu Oct 10 03:31:54 UTC 2013


On Wed, Oct 2, 2013 at 3:35 AM, Jonathan Liu <net147 at gmail.com> wrote:
...
> +  dos: improve MBR signature generation by extracting the
> +  generate_random_uint32 function used for generating FAT serial numbers
> +  into a common header and reusing it.  Previously tv_usec in struct
> +  timeval from gettimeofday() was used which didn't provide enough
> +  precision to fill an unsigned 32-bit integer and wasn't really random.
> +  A check is also added to avoid returning zero which may be interpreted
> +  as no FAT serial number or no MBR signature.
...

Thanks again.
I've tested that patch and shortened/revised your NEWS entry to this:

  dos: the range of random MBR signature values was artificially limited
  to 0..999999, which mistakenly included 0.  Now, we use the full 32-bit
  range, but exclude 0.

I'll push the attached tomorrow.
-------------- next part --------------
From 70aa35b2b4d2e723fe82ac3184e5921a52be73ab Mon Sep 17 00:00:00 2001
From: Jonathan Liu <net147 at gmail.com>
Date: Fri, 4 Oct 2013 07:32:12 -0700
Subject: [PATCH 1/2] dos: improve MBR signature generation

Using tv_usec in struct timeval from gettimeofday() doesn't provide
enough precision to fill an unsigned 32-bit integer and isn't really
random. It it always less than one million when using the GNU C library
while an unsigned 32-bit integer ranges between 0 and 4294967295.

In FAT filesystem creation, parted already uses a better random
generator, so move that code into a common function and use it
for MS-DOS MBR signature generation.

* libparted/fs/r/fat/fat.c (_gen_new_serial_number): Remove.
(fat_create): Use generate_random_uint32 instead of
_gen_new_serial_number.
* libparted/labels/dos.c (generate_random_id): Remove.
(msdos_write): Use generate_random_uint32 instead of
generate_random_id.
* libparted/labels/misc.h (generate_random_uint32): New function.
Created from _gen_new_serial_number in libparted/fs/r/fat/fat.c with
additional check to avoid returning zero, which may be interpreted
as no FAT serial number or no MBR signature.
---
 NEWS                     |  4 ++++
 libparted/fs/r/fat/fat.c | 19 ++-----------------
 libparted/labels/dos.c   | 12 +-----------
 libparted/labels/misc.h  | 21 +++++++++++++++++++++
 4 files changed, 28 insertions(+), 28 deletions(-)

diff --git a/NEWS b/NEWS
index 98f7c6e..50faf4d 100644
--- a/NEWS
+++ b/NEWS
@@ -23,6 +23,10 @@ GNU parted NEWS                                    -*- outline -*-
   partprobe now tells the kernel to forget about any partitions
   on a device that has no recognizable partition table.

+  dos: the range of random MBR signature values was artificially limited
+  to 0..999999, which mistakenly included 0.  Now, we use the full 32-bit
+  range, but exclude 0.
+
 ** Changes in behavior

   parted -l no longer lists device-mapper devices other than
diff --git a/libparted/fs/r/fat/fat.c b/libparted/fs/r/fat/fat.c
index 2ab9279..c8e4552 100644
--- a/libparted/fs/r/fat/fat.c
+++ b/libparted/fs/r/fat/fat.c
@@ -18,10 +18,10 @@

 #include <config.h>
 #include <string.h>
-#include <uuid/uuid.h>

 #include "fat.h"
 #include "calc.h"
+#include "../../../labels/misc.h"

 PedFileSystem*
 fat_alloc (const PedGeometry* geom)
@@ -202,21 +202,6 @@ fat_root_dir_clear (PedFileSystem* fs)
 				   fs_info->root_dir_sector_count);
 }

-/* hack: use the ext2 uuid library to generate a reasonably random (hopefully
- * with /dev/random) number.  Unfortunately, we can only use 4 bytes of it
- */
-static uint32_t
-_gen_new_serial_number (void)
-{
-	union {
-		uuid_t uuid;
-		uint32_t i;
-	} uu32;
-
-	uuid_generate (uu32.uuid);
-	return uu32.i;
-}
-
 PedFileSystem*
 fat_create (PedGeometry* geom, FatType fat_type, PedTimer* timer)
 {
@@ -316,7 +301,7 @@ fat_create (PedGeometry* geom, FatType fat_type, PedTimer* timer)
 			return 0;
 	}

-	fs_info->serial_number = _gen_new_serial_number ();
+	fs_info->serial_number = generate_random_uint32 ();

 	if (!fat_boot_sector_set_boot_code (&fs_info->boot_sector))
 		goto error_free_buffers;
diff --git a/libparted/labels/dos.c b/libparted/labels/dos.c
index b8c161f..6bddd79 100644
--- a/libparted/labels/dos.c
+++ b/libparted/labels/dos.c
@@ -1236,16 +1236,6 @@ write_extended_partitions (const PedDisk* disk)
 		return write_empty_table (disk, ext_part->geom.start);
 }

-static inline uint32_t generate_random_id (void)
-{
-	struct timeval tv;
-	int rc;
-	rc = gettimeofday(&tv, NULL);
-	if (rc == -1)
-		return 0;
-	return (uint32_t)(tv.tv_usec & 0xFFFFFFFFUL);
-}
-
 static int
 msdos_write (const PedDisk* disk)
 {
@@ -1267,7 +1257,7 @@ msdos_write (const PedDisk* disk)

 	/* If there is no unique identifier, generate a random one */
 	if (!table->mbr_signature)
-		table->mbr_signature = generate_random_id();
+		table->mbr_signature = generate_random_uint32 ();

 	memset (table->partitions, 0, sizeof (table->partitions));
 	table->magic = PED_CPU_TO_LE16 (MSDOS_MAGIC);
diff --git a/libparted/labels/misc.h b/libparted/labels/misc.h
index c2ccea1..c039c5f 100644
--- a/libparted/labels/misc.h
+++ b/libparted/labels/misc.h
@@ -16,6 +16,27 @@
     You should have received a copy of the GNU General Public License
     along with this program.  If not, see <http://www.gnu.org/licenses/>. */

+#include <inttypes.h>
+#include <uuid/uuid.h>
+
+/* hack: use the ext2 uuid library to generate a reasonably random (hopefully
+ * with /dev/random) number.  Unfortunately, we can only use 4 bytes of it.
+ * We make sure to avoid returning zero which may be interpreted as no FAT
+ * serial number or no MBR signature.
+ */
+static inline uint32_t
+generate_random_uint32 (void)
+{
+       union {
+               uuid_t uuid;
+               uint32_t i;
+       } uu32;
+
+       uuid_generate (uu32.uuid);
+
+       return uu32.i > 0 ? uu32.i : 0xffffffff;
+}
+
 /* Return nonzero if FS_TYPE_NAME starts with "linux-swap".
    This must match the NUL-terminated "linux-swap" as well
    as "linux-swap(v0)" and "linux-swap(v1)".  */
-- 
1.8.4.299.gb3e7d24


From 9b8f632e102c0d9e2187f0c8d8205862540cdcd1 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering at fb.com>
Date: Wed, 9 Oct 2013 17:44:05 -0700
Subject: [PATCH 2/2] bootstrap: update to latest from gnulib

---
 bootstrap | 159 ++++++++++++++++++++++++++++++++++----------------------------
 1 file changed, 87 insertions(+), 72 deletions(-)

diff --git a/bootstrap b/bootstrap
index 48181c9..e31d17d 100755
--- a/bootstrap
+++ b/bootstrap
@@ -1,6 +1,6 @@
 #! /bin/sh
 # Print a version string.
-scriptversion=2012-07-19.14; # UTC
+scriptversion=2013-08-15.22; # UTC

 # Bootstrap this package from checked-out sources.

@@ -140,20 +140,21 @@ po_download_command_format2=\
 "wget --mirror -nd -q -np -A.po -P '%s' \
  http://translationproject.org/latest/%s/"

+# Prefer a non-empty tarname (4th argument of AC_INIT if given), else
+# fall back to the package name (1st argument with munging)
 extract_package_name='
-  /^AC_INIT(/{
-     /.*,.*,.*, */{
-       s///
-       s/[][]//g
-       s/)$//
+  /^AC_INIT(\[*/{
+     s///
+     /^[^,]*,[^,]*,[^,]*,[ []*\([^][ ,)]\)/{
+       s//\1/
+       s/[],)].*//
        p
        q
      }
-     s/AC_INIT(\[*//
-     s/]*,.*//
+     s/[],)].*//
      s/^GNU //
      y/ABCDEFGHIJKLMNOPQRSTUVWXYZ/abcdefghijklmnopqrstuvwxyz/
-     s/[^A-Za-z0-9_]/-/g
+     s/[^abcdefghijklmnopqrstuvwxyz0123456789_]/-/g
      p
   }
 '
@@ -208,12 +209,16 @@ bootstrap_sync=false
 # Use git to update gnulib sources
 use_git=true

+check_exists() {
+  ($1 --version </dev/null) >/dev/null 2>&1
+  test $? -lt 126
+}
+
 # find_tool ENVVAR NAMES...
 # -------------------------
 # Search for a required program.  Use the value of ENVVAR, if set,
-# otherwise find the first of the NAMES that can be run (i.e.,
-# supports --version).  If found, set ENVVAR to the program name,
-# die otherwise.
+# otherwise find the first of the NAMES that can be run.
+# If found, set ENVVAR to the program name, die otherwise.
 #
 # FIXME: code duplication, see also gnu-web-doc-update.
 find_tool ()
@@ -223,27 +228,21 @@ find_tool ()
   find_tool_names=$@
   eval "find_tool_res=\$$find_tool_envvar"
   if test x"$find_tool_res" = x; then
-    for i
-    do
-      if ($i --version </dev/null) >/dev/null 2>&1; then
-       find_tool_res=$i
-       break
+    for i; do
+      if check_exists $i; then
+        find_tool_res=$i
+        break
       fi
     done
-  else
-    find_tool_error_prefix="\$$find_tool_envvar: "
   fi
-  test x"$find_tool_res" != x \
-    || die "one of these is required: $find_tool_names"
-  ($find_tool_res --version </dev/null) >/dev/null 2>&1 \
-    || die "${find_tool_error_prefix}cannot run $find_tool_res --version"
+  if test x"$find_tool_res" = x; then
+    warn_ "one of these is required: $find_tool_names;"
+    die   "alternatively set $find_tool_envvar to a compatible tool"
+  fi
   eval "$find_tool_envvar=\$find_tool_res"
   eval "export $find_tool_envvar"
 }

-# Find sha1sum, named gsha1sum on MacPorts, and shasum on Mac OS X 10.6.
-find_tool SHA1SUM sha1sum gsha1sum shasum
-
 # Override the default configuration, if necessary.
 # Make sure that bootstrap.conf is sourced from the current directory
 # if we were invoked as "sh bootstrap".
@@ -255,12 +254,12 @@ esac
 # Extra files from gnulib, which override files from other sources.
 test -z "${gnulib_extra_files}" && \
   gnulib_extra_files="
-        $build_aux/install-sh
-        $build_aux/mdate-sh
-        $build_aux/texinfo.tex
-        $build_aux/depcomp
-        $build_aux/config.guess
-        $build_aux/config.sub
+        build-aux/install-sh
+        build-aux/mdate-sh
+        build-aux/texinfo.tex
+        build-aux/depcomp
+        build-aux/config.guess
+        build-aux/config.sub
         doc/INSTALL
 "

@@ -306,34 +305,34 @@ if test -n "$checkout_only_file" && test ! -r "$checkout_only_file"; then
   die "Bootstrapping from a non-checked-out distribution is risky."
 fi

-# Ensure that lines starting with ! sort last, per gitignore conventions
-# for whitelisting exceptions after a more generic blacklist pattern.
-sort_patterns() {
-  sort -u "$@" | sed '/^!/ {
-    H
-    d
-  }
-  $ {
-    P
-    x
-    s/^\n//
-  }' | sed '/^$/d'
+# Strip blank and comment lines to leave significant entries.
+gitignore_entries() {
+  sed '/^#/d; /^$/d' "$@"
 }

-# If $STR is not already on a line by itself in $FILE, insert it,
-# sorting the new contents of the file and replacing $FILE with the result.
-insert_sorted_if_absent() {
+# If $STR is not already on a line by itself in $FILE, insert it at the start.
+# Entries are inserted at the start of the ignore list to ensure existing
+# entries starting with ! are not overridden.  Such entries support
+# whitelisting exceptions after a more generic blacklist pattern.
+insert_if_absent() {
   file=$1
   str=$2
   test -f $file || touch $file
-  echo "$str" | sort_patterns - $file | cmp -s - $file > /dev/null \
-    || { echo "$str" | sort_patterns - $file > $file.bak \
-      && mv $file.bak $file; } \
-    || die "insert_sorted_if_absent $file $str: failed"
+  test -r $file || die "Error: failed to read ignore file: $file"
+  duplicate_entries=$(gitignore_entries $file | sort | uniq -d)
+  if [ "$duplicate_entries" ] ; then
+    die "Error: Duplicate entries in $file: " $duplicate_entries
+  fi
+  linesold=$(gitignore_entries $file | wc -l)
+  linesnew=$( { echo "$str"; cat $file; } | gitignore_entries | sort -u | wc -l)
+  if [ $linesold != $linesnew ] ; then
+    { echo "$str" | cat - $file > $file.bak && mv $file.bak $file; } \
+      || die "insert_if_absent $file $str: failed"
+  fi
 }

 # Adjust $PATTERN for $VC_IGNORE_FILE and insert it with
-# insert_sorted_if_absent.
+# insert_if_absent.
 insert_vc_ignore() {
   vc_ignore_file="$1"
   pattern="$2"
@@ -344,7 +343,7 @@ insert_vc_ignore() {
     # .gitignore entry.
     pattern=$(echo "$pattern" | sed s,^,/,);;
   esac
-  insert_sorted_if_absent "$vc_ignore_file" "$pattern"
+  insert_if_absent "$vc_ignore_file" "$pattern"
 }

 # Die if there is no AC_CONFIG_AUX_DIR($build_aux) line in configure.ac.
@@ -468,8 +467,7 @@ check_versions() {
     if [ "$req_ver" = "-" ]; then
       # Merely require app to exist; not all prereq apps are well-behaved
       # so we have to rely on $? rather than get_version.
-      $app --version >/dev/null 2>&1
-      if [ 126 -le $? ]; then
+      if ! check_exists $app; then
         warn_ "Error: '$app' not found"
         ret=1
       fi
@@ -502,6 +500,12 @@ print_versions() {
   # can't depend on column -t
 }

+# Find sha1sum, named gsha1sum on MacPorts, shasum on Mac OS X 10.6.
+# Also find the compatible sha1 utility on the BSDs
+if test x"$SKIP_PO" = x; then
+  find_tool SHA1SUM sha1sum gsha1sum shasum sha1
+fi
+
 use_libtool=0
 # We'd like to use grep -E, to see if any of LT_INIT,
 # AC_PROG_LIBTOOL, AM_PROG_LIBTOOL is used in configure.ac,
@@ -550,10 +554,10 @@ fi
 echo "$0: Bootstrapping from checked-out $package sources..."

 # See if we can use gnulib's git-merge-changelog merge driver.
-if test -d .git && (git --version) >/dev/null 2>/dev/null ; then
+if $use_git && test -d .git && check_exists git; then
   if git config merge.merge-changelog.driver >/dev/null ; then
     :
-  elif (git-merge-changelog --version) >/dev/null 2>/dev/null ; then
+  elif check_exists git-merge-changelog; then
     echo "$0: initializing git-merge-changelog driver"
     git config merge.merge-changelog.name 'GNU-style ChangeLog merge driver'
     git config merge.merge-changelog.driver 'git-merge-changelog %O %A %B'
@@ -573,13 +577,17 @@ git_modules_config () {
   test -f .gitmodules && git config --file .gitmodules "$@"
 }

-gnulib_path=$(git_modules_config submodule.gnulib.path)
-test -z "$gnulib_path" && gnulib_path=gnulib
+if $use_git; then
+  gnulib_path=$(git_modules_config submodule.gnulib.path)
+  test -z "$gnulib_path" && gnulib_path=gnulib
+fi

-# Get gnulib files.
+# Get gnulib files.  Populate $GNULIB_SRCDIR, possibly updating a
+# submodule, for use in the rest of the script.

 case ${GNULIB_SRCDIR--} in
 -)
+  # Note that $use_git is necessarily true in this case.
   if git_modules_config submodule.gnulib.url >/dev/null; then
     echo "$0: getting gnulib files..."
     git submodule init || exit $?
@@ -600,8 +608,8 @@ case ${GNULIB_SRCDIR--} in
   GNULIB_SRCDIR=$gnulib_path
   ;;
 *)
-  # Use GNULIB_SRCDIR as a reference.
-  if test -d "$GNULIB_SRCDIR"/.git && \
+  # Use GNULIB_SRCDIR directly or as a reference.
+  if $use_git && test -d "$GNULIB_SRCDIR"/.git && \
         git_modules_config submodule.gnulib.url >/dev/null; then
     echo "$0: getting gnulib files..."
     if git submodule -h|grep -- --reference > /dev/null; then
@@ -627,12 +635,19 @@ case ${GNULIB_SRCDIR--} in
   ;;
 esac

+# $GNULIB_SRCDIR now points to the version of gnulib to use, and
+# we no longer need to use git or $gnulib_path below here.
+
 if $bootstrap_sync; then
   cmp -s "$0" "$GNULIB_SRCDIR/build-aux/bootstrap" || {
     echo "$0: updating bootstrap and restarting..."
+    case $(sh -c 'echo "$1"' -- a) in
+      a) ignored=--;;
+      *) ignored=ignored;;
+    esac
     exec sh -c \
       'cp "$1" "$2" && shift && exec "${CONFIG_SHELL-/bin/sh}" "$@"' \
-      -- "$GNULIB_SRCDIR/build-aux/bootstrap" \
+      $ignored "$GNULIB_SRCDIR/build-aux/bootstrap" \
       "$0" "$@" --no-bootstrap-sync
   }
 fi
@@ -680,11 +695,10 @@ update_po_files() {
     cksum_file="$ref_po_dir/$po.s1"
     if ! test -f "$cksum_file" ||
         ! test -f "$po_dir/$po.po" ||
-        ! $SHA1SUM -c --status "$cksum_file" \
-            < "$new_po" > /dev/null; then
+        ! $SHA1SUM -c "$cksum_file" < "$new_po" > /dev/null 2>&1; then
       echo "$me: updated $po_dir/$po.po..."
       cp "$new_po" "$po_dir/$po.po" \
-          && $SHA1SUM < "$new_po" > "$cksum_file"
+          && $SHA1SUM < "$new_po" > "$cksum_file" || return
     fi
   done
 }
@@ -889,20 +903,21 @@ find "$m4_base" "$source_base" \
   -depth \( -name '*.m4' -o -name '*.[ch]' \) \
   -type l -xtype l -delete > /dev/null 2>&1

+# Invoke autoreconf with --force --install to ensure upgrades of tools
+# such as ylwrap.
+AUTORECONFFLAGS="--verbose --install --force -I $m4_base $ACLOCAL_FLAGS"
+
 # Some systems (RHEL 5) are using ancient autotools, for which the
 # --no-recursive option had not been invented.  Detect that lack and
 # omit the option when it's not supported.  FIXME in 2017: remove this
 # hack when RHEL 5 autotools are updated, or when they become irrelevant.
-no_recursive=
 case $($AUTORECONF --help) in
-  *--no-recursive*) no_recursive=--no-recursive;;
+  *--no-recursive*) AUTORECONFFLAGS="$AUTORECONFFLAGS --no-recursive";;
 esac

 # Tell autoreconf not to invoke autopoint or libtoolize; they were run above.
-echo "running: AUTOPOINT=true LIBTOOLIZE=true " \
-    "$AUTORECONF --verbose --install $no_recursive -I $m4_base $ACLOCAL_FLAGS"
-AUTOPOINT=true LIBTOOLIZE=true \
-    $AUTORECONF --verbose --install $no_recursive -I $m4_base $ACLOCAL_FLAGS \
+echo "running: AUTOPOINT=true LIBTOOLIZE=true $AUTORECONF $AUTORECONFFLAGS"
+AUTOPOINT=true LIBTOOLIZE=true $AUTORECONF $AUTORECONFFLAGS \
   || die "autoreconf failed"

 # Get some extra files from gnulib, overriding existing files.
-- 
1.8.4.299.gb3e7d24



More information about the parted-devel mailing list