WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-changelog

[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

<Prev in Thread] Current Thread [Next in Thread>
  • [Xen-changelog] Many fixes and cleanups for lomount:, Xen patchbot -unstable <=