[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
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |