[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-changelog] Many fixes and cleanups for lomount:



# HG changeset patch
# User kaf24@xxxxxxxxxxxxxxxxxxxx
# Node ID dfa7ba9c1296c232ceced197cee88f9d02237a68
# Parent  fd102a15436fba92dd132e1fc7bbe38bb180444d
Many fixes and cleanups for lomount:
- Fixed several overflows, off-by-one, and uninitialized variables.
- Added well-defined exit codes.
- Proper handling of system()'s return value.
- Errors parsing partition table cause it to stop now.
- etcetera...

Tested on 32 and 64 bit, with valgrind, with physical disks and disk
images.

Signed-off-by: Charles Coffing <ccoffing@xxxxxxxxxx>

diff -r fd102a15436f -r dfa7ba9c1296 tools/misc/lomount/lomount.c
--- a/tools/misc/lomount/lomount.c      Thu Mar  2 20:34:16 2006
+++ b/tools/misc/lomount/lomount.c      Thu Mar  2 20:35:17 2006
@@ -24,16 +24,33 @@
  * THE SOFTWARE.
  */
 
+/*
+ *  Return code:
+ *
+ *  bit 7 set:         lomount wrapper failed
+ *  bit 7 clear:       lomount wrapper ok; mount's return code in low 7 bits
+ *  0                  success
+ */
+
+enum
+{
+       ERR_USAGE = 0x80,       // Incorrect usage
+       ERR_PART_PARSE,         // Failed to parse partition table
+       ERR_NO_PART,            // No such partition
+       ERR_NO_EPART,           // No such extended partition
+       ERR_MOUNT               // Other failure of mount command
+};
+
 #include <unistd.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 #include <strings.h>
+#include <sys/wait.h>
 #include <errno.h>
 
 #define BUF 4096
 
-//#define SECSIZE 4096    /* arbitrarilly large (it's probably just 512) */
 #define SECSIZE 512
 
 struct pentry 
@@ -50,30 +67,32 @@
        unsigned long no_of_sectors_abs;
 };
 
-char * progname;
-
-int loadptable(const char *argv, struct pentry parttbl[], struct pentry 
**exttbl)
-{
-       FILE *fd; 
-       int i, valid, total_known_sectors = 0;
+int loadptable(const char *diskimage, struct pentry parttbl[], struct pentry 
**exttbl, int * valid)
+{
+       FILE *fd;
+       size_t size;
+       int fail = 1;
+       int i, total_known_sectors = 0;
        unsigned char *pi; 
        unsigned char data [SECSIZE]; 
        unsigned long extent = 0, old_extent = 0, e_count = 1;
        struct pentry exttbls[4];
 
-       fd = fopen(argv, "r");
+       *valid = 0;
+
+       fd = fopen(diskimage, "r");
        if (fd == NULL)
        {
-               perror ("lomount"); 
-               return 1;
-       }
-       i = fread (&data, 1, sizeof (data), fd); 
-       if (i < SECSIZE)
-       {
-               fprintf (stderr, "%s: could not read the entire first 
sector\n", progname); 
-               return 1;
-       }
-       for (i = 0;i < 4;i++)
+               perror(diskimage);
+               goto done;
+       }
+       size = fread (&data, 1, sizeof(data), fd);
+       if (size < (size_t)sizeof(data))
+       {
+               fprintf(stderr, "Could not read the entire first sector of 
%s.\n", diskimage);
+               goto done;
+       }
+       for (i = 0; i < 4; i++)
        {
                pi = &data [446 + 16 * i];
                parttbl [i].bootable = *pi; 
@@ -95,7 +114,7 @@
                        old_extent = extent;
                }
        }
-       valid = (data [510] == 0x55 && data [511] == 0xaa) ? 1 : 0;
+       *valid = (data [510] == 0x55 && data [511] == 0xaa) ? 1 : 0;
        for (i = 0; i < 4; i++)
        {
                total_known_sectors += parttbl[i].no_of_sectors_abs;
@@ -105,7 +124,7 @@
 #ifdef DEBUG
        if (extent != 0)
        {
-               printf("extended partition detected at offset %d\n", extent);
+               printf("extended partition detected at offset %ld\n", extent);
        }
 #endif
        while (extent != 0)
@@ -113,14 +132,14 @@
 /* according to realloc(3) passing NULL as pointer is same as calling malloc() 
*/
                exttbl[0] = realloc(exttbl[0], e_count * sizeof(struct pentry));
                fseek(fd, extent, SEEK_SET);
-               i = fread (&data, 1, sizeof (data), fd); 
-               if (i < SECSIZE)
-               {
-                       fprintf (stderr, "%s: could not read the entire first 
sector\n", progname); 
-                       return 1;
+               size = fread (&data, 1, sizeof(data), fd);
+               if (size < (size_t)sizeof(data))
+               {
+                       fprintf(stderr, "Could not read extended partition of 
%s.", diskimage);
+                       goto done;
                }
        /* only first 2 entrys are used in extented partition tables */
-               for (i = 0;i < 2;i++)
+               for (i = 0; i < 2; i++)
                {
                        pi = &data [446 + 16 * i];
                        exttbls [i].bootable = *pi; 
@@ -152,7 +171,7 @@
                        /* adjust for start of image instead of start of ext 
partition */
                                exttbl[0][e_count-1].start_sector_abs += 
(extent/SECSIZE);
 #ifdef DEBUG
-                               printf("extent %d start_sector_abs %d\n", 
extent, exttbl[0][e_count-1].start_sector_abs);
+                               printf("extent %ld start_sector_abs %ld\n", 
extent, exttbl[0][e_count-1].start_sector_abs);
 #endif
                        //else if (parttbl[i].system == 0x5)
                        }
@@ -165,53 +184,71 @@
                }
                e_count ++;
        }
-       //fclose (fd);
-       //the above segfaults (?!!!)
-#ifdef DEBUG
-       printf("e_count = %d\n", e_count);
-#endif
-       return valid;
+#ifdef DEBUG
+       printf("e_count = %ld\n", e_count);
+#endif
+       fail = 0;
+
+done:
+       if (fd)
+               fclose(fd);
+       return fail;
 }
 
+void usage()
+{
+       fprintf(stderr, "You must specify at least -diskimage and 
-partition.\n");
+       fprintf(stderr, "All other arguments are passed through to 'mount'.\n");
+       fprintf(stderr, "ex. lomount -t fs-type -diskimage hda.img -partition 1 
/mnt\n");
+       exit(ERR_USAGE);
+}
+
 int main(int argc, char ** argv)
 {
+       int status;
        struct pentry perttbl [4];
        struct pentry *exttbl[1], *parttbl;
-       char buf[BUF], argv2[BUF], diskimage[BUF];
-       int partition = 1, sec, num = 0, pnum = 0, len = BUF, i, f = 0, valid;
-       progname = argv[0];
+       char buf[BUF], argv2[BUF];
+       const char * diskimage = 0;
+       int partition = 0, sec, num = 0, pnum = 0, i, valid;
+       size_t argv2_len = sizeof(argv2);
+       argv2[0] = '\0';
        exttbl[0] = NULL;
+
        for (i = 1; i < argc; i ++)
        {
-               if (strncmp(argv[i], "-diskimage", BUF)==0)
-               {
-                       strncpy(diskimage, argv[i+1], BUF);
-                       i++; f = 1;
-               }
-               else if (strncmp(argv[i], "-partition", BUF)==0)
-               {
-                       partition = atoi(argv[i+1]);
+               if (strcmp(argv[i], "-diskimage")==0)
+               {
+                       if (i == argc-1)
+                               usage();
                        i++;
-                       if (partition < 1) partition = 1;
+                       diskimage = argv[i];
+               }
+               else if (strcmp(argv[i], "-partition")==0)
+               {
+                       if (i == argc-1)
+                               usage();
+                       i++;
+                       partition = atoi(argv[i]);
                }
                else
                {
-                       strncat(argv2, argv[i], len);
-                       strncat(argv2, " ", len-1);
-                       len -= strlen(argv[i]);
-                       len--;
-               }
-       }
-       if (!f)
-       {
-               printf("You must specify -diskimage and -partition\n");
-               printf("ex. lomount -t fs-type -diskimage hda.img -partition 1 
/mnt\n");
-               return 0;
-       }
-       valid = loadptable(diskimage, perttbl, exttbl);
+                       size_t len = strlen(argv[i]);
+                       if (len >= argv2_len-1)
+                               usage();
+                       strcat(argv2, argv[i]);
+                       strcat(argv2, " ");
+                       len -= (len+1);
+               }
+       }
+       if (! diskimage || partition < 1)
+               usage();
+
+       if (loadptable(diskimage, perttbl, exttbl, &valid))
+               return ERR_PART_PARSE;
        if (!valid)
        {
-               printf("Warning: disk image does not appear to describe a valid 
partition table.\n");
+               fprintf(stderr, "Warning: disk image does not appear to 
describe a valid partition table.\n");
        }
        /* NOTE: need to make sure this always rounds down */
        //sec = total_known_sectors / sizeof_diskimage;
@@ -228,14 +265,14 @@
        {
                if (exttbl[0] == NULL)
                {
-                   printf("No extended partitions were found in %s.\n", 
diskimage);
-                   return 2;
+                   fprintf(stderr, "No extended partitions were found in 
%s.\n", diskimage);
+                   return ERR_NO_EPART;
                }
                parttbl = exttbl[0];
                if (parttbl[partition-5].no_of_sectors_abs == 0)
                {
-                       printf("Partition %d was not found in %s.\n", 
partition, diskimage);
-                       return 3;
+                       fprintf(stderr, "Partition %d was not found in %s.\n", 
partition, diskimage);
+                       return ERR_NO_PART;
                }
                partition -= 4;
        }
@@ -244,8 +281,8 @@
                parttbl = perttbl;
                if (parttbl[partition-1].no_of_sectors_abs == 0)
                {
-                       printf("Partition %d was not found in %s.\n", 
partition, diskimage);
-                       return 3;
+                       fprintf(stderr, "Partition %d was not found in %s.\n", 
partition, diskimage);
+                       return ERR_NO_PART;
                }
        }
        num = parttbl[partition-1].start_sector_abs;
@@ -253,10 +290,14 @@
 #ifdef DEBUG
        printf("offset = %d\n", pnum);
 #endif
-       snprintf(buf, BUF, "mount -oloop,offset=%d %s %s", pnum, diskimage, 
argv2);
+       snprintf(buf, sizeof(buf), "mount -oloop,offset=%d %s %s", pnum, 
diskimage, argv2);
 #ifdef DEBUG
        printf("%s\n", buf);
 #endif
-       system(buf);
-       return 0;
+       status = system(buf);
+       if (WIFEXITED(status))
+               status = WEXITSTATUS(status);
+       else
+               status = ERR_MOUNT;
+       return status;
 }

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.