NetBSD-Bugs archive

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

bin/45749: disklabel(8) uses wrong boundaries for several values, destroys disklabels



>Number:         45749
>Category:       bin
>Synopsis:       disklabel(8) uses wrong boundaries for several values, 
>destroys disklabels
>Confidential:   no
>Severity:       serious
>Priority:       high
>Responsible:    bin-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Dec 27 21:10:00 +0000 2011
>Originator:     Julian Fagir
>Release:        NetBSD-current
>Organization:
>Environment:
>Description:
disklabel(8) in interactional mode sets several values too high.

I think, it should be d_npartitions, d_interleave, d_trackskew, d_cylskew being 
defined as uint16_t, but imported as SCNu32. Specifying a large value simply 
messes up your disklabel.

I tried replacing all SCNu32 of the inappropriate values of SCNu16 (see 
attached diff). This solves formal issues, but neither the segfault nor does it 
check for the right values.

While I didn't track down the segfault, there's another obvious flaw: The input 
is simply scanf'ed. There is no check whether the input value is *really* the 
value typed in - if you type in a value too large for 16 uint (or for 32 uint), 
then simply the highest value is taken.
If you type in bogus stuff, but scanf succeeds, there is no warning nor error, 
only the found integer is taken.
>How-To-Repeat:
# disklabel -i wd1
Enter '?' for help
partition> P
4 partitions:
#        size    offset     fstype [fsize bsize cpg/sgs]
 c:   2088387        63     unused      0     0        # (Cyl.      0*-   2071*)
 d:   4194304         0     unused      0     0        # (Cyl.      0 -   4161*)
partition> I
# Current values:
# /dev/rwd1d:
type: ESDI
disk: VBOX HARDDISK
label: fictitious
flags:
bytes/sector: 512
sectors/track: 63
tracks/cylinder: 16
sectors/cylinder: 1008
cylinders: 4161
total sectors: 4194304
rpm: 3600
interleave: 1
trackskew: 0
cylinderskew: 0
headswitch: 0           # microseconds
track-to-track seek: 0  # microseconds
drivedata: 0

Disk type [?] [ESDI]:
Disk name [VBOX HARDDISK   ]:
Label name [fictitious]:
Number of partitions [4]: 4294967295
Sector size (bytes) [512]:
Number of sectors per track [63]:
Number of tracks per cylinder [16]:
Number of sectors/cylinder [1008]:
Total number of cylinders [4161]:
Total number of sectors [4194304]:
Hardware sectors interleave [1]:
Sector 0 skew, per track [0]:
Sector 0 skew, per cylinder [0]:
Head switch time (usec) [0]:
Track seek time (usec) [0]:
partition> P
65535 partitions:
#        size    offset     fstype [fsize bsize cpg/sgs]
 c:   2088387        63     unused      0     0        # (Cyl.      0*-   2071*)
 d:   4194304         0     unused      0     0        # (Cyl.      0 -   4161*)
 x: 1685549615     25649     unused      0     0        # (Cyl.     25*- 
1672197+)
 º: 134556356         0     unused      0     0        # (Cyl.      0 - 133488+)
 Á:      4096      1914    ISO9660      60             # (Cyl.      1*-      5*)
 Ã:       986        31     unused    152     0        # (Cyl.      0*-      1*)
 Å:        16        96     unused      0     0        # (Cyl.      0*-      0*)
 Æ:        16      4096      MSDOS                     # (Cyl.      4*-      4*)
 Ç:        64         0         32                     # (Cyl.      0 -      0*)
 È:      4096       126         64                     # (Cyl.      0*-      4*)
 Ê:        84         3     unused     64     0        # (Cyl.      0*-      0*)
 Ì:         2        64     unused      0     0        # (Cyl.      0*-      0*)
 Í:        80      4096  Version 6                     # (Cyl.      4*-      4*)
 Î:        96         0         96                     # (Cyl.      0 -      0*)
 Ï:      8192        84        128                     # (Cyl.      0*-      8*)
 Ñ:        72         3     unused    128     0        # (Cyl.      0*-      0*)
 Ó:         2       128     unused      0     0        # (Cyl.      0*-      0*)
 Ô:       144      8192  Version 6                     # (Cyl.      8*-      8*)
 Õ:       128         0        160                     # (Cyl.      0 -      0*)
 Ö:      8192        50        192                     # (Cyl.      0*-      8*)
 Ø:        46         2     unused     96     0        # (Cyl.      0*-      0*)
 Ú:         2       192     unused      0     0        # (Cyl.      0*-      0*)
 Û:       208     12288  Version 6                     # (Cyl.     12*-     12*)
 Ü:       224         0        224                     # (Cyl.      0 -      0*)
 Ý:     16384        72     unused      3     3        # (Cyl.      0*-     16*)
 ß:        68         3     unused     64     0        # (Cyl.      0*-      0*)
 á:         2       256     unused      0     0        # (Cyl.      0*-      0*)
 â:       272     16384  Version 6                     # (Cyl.     16*-     16*)
 ã:        64         0         32                     # (Cyl.      0 -      0*)
 ä:     20480        70         64                     # (Cyl.      0*-     20*)
 æ:        67         3     unused    112     0        # (Cyl.      0*-      0*)
 è:         2       320     unused      0     0        # (Cyl.      0*-      0*)
 é:       336     20480  Version 6                     # (Cyl.     20*-     20*)
 ê:       320         0         96                     # (Cyl.      0 -      0*)
 ë:     20480        58         64                     # (Cyl.      0*-     20*)
 í:        55         2     unused    240     0        # (Cyl.      0*-      0*)
 ï:         2       384     unused      0     0        # (Cyl.      0*-      0*)
 ð:       400     24576  Version 6                     # (Cyl.     24*-     24*)
 ñ:       176         0        160                     # (Cyl.      0 -      0*)
 ò:     24576        58        192                     # (Cyl.      0*-     24*)
 ô:        56         2     unused    384     0        # (Cyl.      0*-      0*)
 ö:         2       448     unused      0     0        # (Cyl.      0*-      0*)
 ÷:       464     28672  Version 6                     # (Cyl.     28*-     28*)
 ø:       368         0        224                     # (Cyl.      0 -      0*)
 ù:     28672        59         96                     # (Cyl.      0*-     28*)
 û:        65         3     unused    528     0        # (Cyl.      0*-      0*)
 ý:         2       512     unused      0     0        # (Cyl.      0*-      0*)
 þ:      1024     32768       swap                     # (Cyl.     32*-     33*)
 ÿ:      1024 3146780672     unused      0     0        # (Cyl. 3121806+- 
3121807+)
 :     32768        15     unused      1     8        # (Cyl.      0*-     32*)
[1]   Segmentation fault (core dumped) disklabel -i wd1

>Fix:
The following patch does only solve one problem: It changes the 32 uint 
variable to the name 'u32' and introduces another 16 bit uint 'u16' which is 
used for the 16 bit values. Neither the segfaults nor the checks for right 
values are solved.

--- interact.c
+++ interact.c
@@ -150,11 +150,12 @@
 cmd_info(struct disklabel *lp, char *s, int fd)
 {
        char    line[BUFSIZ];
        char    def[BUFSIZ];
        int     v, i;
-       u_int32_t u;
+       u_int32_t u32;
+       u_int16_t u16;
 
        printf("# Current values:\n");
        showinfo(stdout, lp, specname);
 
        /* d_type */
@@ -207,15 +208,15 @@
                i = getinput(":", "Number of partitions", def, line);
                if (i == -1)
                        return;
                else if (i == 0)
                        break;
-               if (sscanf(line, "%" SCNu32, &u) != 1) {
+               if (sscanf(line, "%" SCNu16, &u16) != 1) {
                        printf("Invalid number of partitions `%s'\n", line);
                        continue;
                }
-               lp->d_npartitions = u;
+               lp->d_npartitions = u16;
                break;
        }
 
        /* d_secsize */
        for (;;) {
@@ -223,15 +224,15 @@
                i = getinput(":", "Sector size (bytes)", def, line);
                if (i == -1)
                        return;
                else if (i == 0)
                        break;
-               if (sscanf(line, "%" SCNu32, &u) != 1) {
+               if (sscanf(line, "%" SCNu32, &u32) != 1) {
                        printf("Invalid sector size `%s'\n", line);
                        continue;
                }
-               lp->d_secsize = u;
+               lp->d_secsize = u32;
                break;
        }
 
        /* d_nsectors */
        for (;;) {
@@ -239,15 +240,15 @@
                i = getinput(":", "Number of sectors per track", def, line);
                if (i == -1)
                        return;
                else if (i == 0)
                        break;
-               if (sscanf(line, "%" SCNu32, &u) != 1) {
+               if (sscanf(line, "%" SCNu32, &u32) != 1) {
                        printf("Invalid number of sectors `%s'\n", line);
                        continue;
                }
-               lp->d_nsectors = u;
+               lp->d_nsectors = u32;
                break;
        }
 
        /* d_ntracks */
        for (;;) {
@@ -255,15 +256,15 @@
                i = getinput(":", "Number of tracks per cylinder", def, line);
                if (i == -1)
                        return;
                else if (i == 0)
                        break;
-               if (sscanf(line, "%" SCNu32, &u) != 1) {
+               if (sscanf(line, "%" SCNu32, &u32) != 1) {
                        printf("Invalid number of tracks `%s'\n", line);
                        continue;
                }
-               lp->d_ntracks = u;
+               lp->d_ntracks = u32;
                break;
        }
 
        /* d_secpercyl */
        for (;;) {
@@ -271,16 +272,16 @@
                i = getinput(":", "Number of sectors/cylinder", def, line);
                if (i == -1)
                        return;
                else if (i == 0)
                        break;
-               if (sscanf(line, "%" SCNu32, &u) != 1) {
+               if (sscanf(line, "%" SCNu32, &u32) != 1) {
                        printf("Invalid number of sector/cylinder `%s'\n",
                            line);
                        continue;
                }
-               lp->d_secpercyl = u;
+               lp->d_secpercyl = u32;
                break;
        }
 
        /* d_ncylinders */
        for (;;) {
@@ -288,15 +289,15 @@
                i = getinput(":", "Total number of cylinders", def, line);
                if (i == -1)
                        return;
                else if (i == 0)
                        break;
-               if (sscanf(line, "%" SCNu32, &u) != 1) {
+               if (sscanf(line, "%" SCNu32, &u32) != 1) {
                        printf("Invalid sector size `%s'\n", line);
                        continue;
                }
-               lp->d_ncylinders = u;
+               lp->d_ncylinders = u32;
                break;
        }
 
        /* d_secperunit */
        for (;;) {
@@ -304,15 +305,15 @@
                i = getinput(":", "Total number of sectors", def, line);
                if (i == -1)
                        return;
                else if (i == 0)
                        break;
-               if (sscanf(line, "%" SCNu32, &u) != 1) {
+               if (sscanf(line, "%" SCNu32, &u32) != 1) {
                        printf("Invalid number of sectors `%s'\n", line);
                        continue;
                }
-               lp->d_secperunit = u;
+               lp->d_secperunit = u32;
                break;
        }
 
        /* d_rpm */
 
@@ -322,15 +323,15 @@
                i = getinput(":", "Hardware sectors interleave", def, line);
                if (i == -1)
                        return;
                else if (i == 0)
                        break;
-               if (sscanf(line, "%" SCNu32, &u) != 1) {
+               if (sscanf(line, "%" SCNu16, &u16) != 1) {
                        printf("Invalid sector interleave `%s'\n", line);
                        continue;
                }
-               lp->d_interleave = u;
+               lp->d_interleave = u16;
                break;
        }
 
        /* d_trackskew */
        for (;;) {
@@ -338,15 +339,15 @@
                i = getinput(":", "Sector 0 skew, per track", def, line);
                if (i == -1)
                        return;
                else if (i == 0)
                        break;
-               if (sscanf(line, "%" SCNu32, &u) != 1) {
+               if (sscanf(line, "%" SCNu16, &u16) != 1) {
                        printf("Invalid track sector skew `%s'\n", line);
                        continue;
                }
-               lp->d_trackskew = u;
+               lp->d_trackskew = u16;
                break;
        }
 
        /* d_cylskew */
        for (;;) {
@@ -354,15 +355,15 @@
                i = getinput(":", "Sector 0 skew, per cylinder", def, line);
                if (i == -1)
                        return;
                else if (i == 0)
                        break;
-               if (sscanf(line, "%" SCNu32, &u) != 1) {
+               if (sscanf(line, "%" SCNu16, &u16) != 1) {
                        printf("Invalid cylinder sector `%s'\n", line);
                        continue;
                }
-               lp->d_cylskew = u;
+               lp->d_cylskew = u16;
                break;
        }
 
        /* d_headswitch */
        for (;;) {
@@ -370,15 +371,15 @@
                i = getinput(":", "Head switch time (usec)", def, line);
                if (i == -1)
                        return;
                else if (i == 0)
                        break;
-               if (sscanf(line, "%" SCNu32, &u) != 1) {
+               if (sscanf(line, "%" SCNu32, &u32) != 1) {
                        printf("Invalid head switch time `%s'\n", line);
                        continue;
                }
-               lp->d_headswitch = u;
+               lp->d_headswitch = u32;
                break;
        }
 
        /* d_trkseek */
        for (;;) {
@@ -386,15 +387,15 @@
                i = getinput(":", "Track seek time (usec)", def, line);
                if (i == -1)
                        return;
                else if (i == 0)
                        break;
-               if (sscanf(line, "%" SCNu32, &u) != 1) {
+               if (sscanf(line, "%" SCNu32, &u32) != 1) {
                        printf("Invalid track seek time `%s'\n", line);
                        continue;
                }
-               lp->d_trkseek = u;
+               lp->d_trkseek = u32;
                break;
        }
 }



Home | Main Index | Thread Index | Old Index