lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Sat, 7 Apr 2007 21:09:36 -0700 From: Randy Dunlap <randy.dunlap@...cle.com> To: "John Anthony Kazos Jr." <jakj@...-k-j.com> Cc: aeb@....nl, linux-kernel@...r.kernel.org Subject: Re: [PATCH 5/5] partitions: Rewrite check_partition to remove necessity of check_part On Sat, 7 Apr 2007 23:42:00 -0400 (EDT) John Anthony Kazos Jr. wrote: > From: John Anthony Kazos Jr. <jakj@...-k-j.com> > > Removes the entire check_part array and uses the presence of new stub > functions in header files in fs/partitions to call them directly in a list > and let the compiler optimize away any that aren't compiled in. Also fixes > a bug where " unable to read partition table" would never be printed. > > Signed-off-by: John Anthony Kazos Jr. <jakj@...-k-j.com> > > --- > > I did this because it seems to be much easier to understand. And even > though the memory used by the array was only sizeof(void*)*n+1 for the > number of partitions configured to be included, that's still unnecessary > usage. > > This code also has two warn_unused_result problems, but I don't feel up to > the task of fixing those yet. > > Within the old loop, if res < 0, res = 0, and immediately after the loop, > the function returns if res > 0. Therefore, it must be that res == 0 after > the loop. if (!err) is true only if err == 0 so again, res == 0, so if > (!res) is true, and the else condition can never be reached. I > re-interpreted the intent to be "log a message if the partition format is > unrecognized, and log a message if the partition cannot be read but only > if the warning is requested". Judging by the "This is ugly" comment, the > warning is not intended to account for actual I/O errors when reading the > partition-table block, but rather to detect when the partitions are > checked for a device with a medium that has been removed. > > --- linux-2.6.20.6-orig/fs/partitions/check.c 2007-04-06 16:02:48.000000000 -0400 > +++ linux-2.6.20.6-mod/fs/partitions/check.c 2007-04-07 21:50:22.000000000 -0400 > @@ -41,73 +41,6 @@ extern void md_autodetect_dev(dev_t dev) > > int warn_no_part = 1; /*This is ugly: should make genhd removable media aware*/ [snip] > /* > * disk_name() is used by partition check code and the genhd driver. > * It formats the devicename of the indicated disk into > @@ -149,11 +82,125 @@ const char *__bdevname(dev_t dev, char * > > EXPORT_SYMBOL(__bdevname); > > +static int > +check_succeeded(int res, struct parsed_partitions *state, int *err) > +{ > + if (res < 0) { > + /* We have hit an I/O error which we don't report now. > + * But record it, and let the others do their job. > + */ > + *err = res; > + res = 0; > + } > + > + if (!res) { > + memset(state->parts, 0, sizeof(state->parts)); > + } No braces on single-statement blocks. (many of these below also) > + > + return res; > +} > + > +static struct parsed_partitions * > +do_check_list(struct parsed_partitions *state, struct block_device *bdev) > +{ > + int err; > + > + err = 0; > + > + /* Use tab to indent (above), not (only) spaces. > + * Probe partition formats with tables at disk address 0 > + * that also have an ADFS boot block at 0xdc0. > + */ > + if (check_succeeded(adfspart_check_ICS(state, bdev), state, &err)) { > + return state; > + } > + if (check_succeeded(adfspart_check_POWERTEC(state, bdev), state, &err)) { > + return state; > + } > + if (check_succeeded(adfspart_check_EESOX(state, bdev), state, &err)) { > + return state; > + } > + > + /* tab above > + * Now move on to formats that only have partition info at > + * disk address 0xdc0. Since these may also have stale > + * PC/BIOS partition tables, they need to come before > + * the msdos entry. > + */ > + if (check_succeeded(adfspart_check_CUMANA(state, bdev), state, &err)) { > + return state; > + } > + if (check_succeeded(adfspart_check_ADFS(state, bdev), state, &err)) { > + return state; > + } > + > + /* this must come before msdos */ > + if (check_succeeded(efi_partition(state, bdev), state, &err)) { > + return state; > + } > + > + if (check_succeeded(sgi_partition(state, bdev), state, &err)) { > + return state; > + } > + > + /* this must come before msdos */ > + if (check_succeeded(ldm_partition(state, bdev), state, &err)) { > + return state; > + } > + > + if (check_succeeded(msdos_partition(state, bdev), state, &err)) { > + return state; > + } > + > + if (check_succeeded(osf_partition(state, bdev), state, &err)) { > + return state; > + } > + > + if (check_succeeded(sun_partition(state, bdev), state, &err)) { > + return state; > + } > + > + if (check_succeeded(amiga_partition(state, bdev), state, &err)) { > + return state; > + } > + > + if (check_succeeded(atari_partition(state, bdev), state, &err)) { > + return state; > + } > + > + if (check_succeeded(mac_partition(state, bdev), state, &err)) { > + return state; > + } > + > + if (check_succeeded(ultrix_partition(state, bdev), state, &err)) { > + return state; > + } > + > + if (check_succeeded(ibm_partition(state, bdev), state, &err)) { > + return state; > + } > + > + if (check_succeeded(karma_partition(state, bdev), state, &err)) { > + return state; > + } > + > + kfree(state); > + > + if (err) { > + if (warn_no_part) { > + printk(" unable to read partition table\n"); > + } > + } else { > + printk(" unknown partition table\n"); > + } > + > + return ERR_PTR(err); > +} --- ~Randy *** Remember to use Documentation/SubmitChecklist when testing your code *** - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists