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