On Saturday 24 October 2009 15:16, Cristian Ionescu-Idbohrn wrote: > On Fri, 23 Oct 2009, Denys Vlasenko wrote: > > > We KNOW bb_path_mtab_file does not contain %format. > > I do not want to vandalize code just because specific version of gcc > > is nuts. > > Sorry, I was neither involved in the development of that specific gcc > option nor in gentoo's (and several other distributions) making > -Wformat-security a default option. Of course, the current behaviour > could be reverted to not being default with a -Wno-format-security in > Makefile.flags. Mike Frysinger wrote about it a while ago: > > http://patchwork.kernel.org/patch/4730/ > > Still, do we want to do that? > > There are some obvious places that would be ok, as we control the strings: > > bb_error_msg(bb_msg_memory_exhausted); > bb_error_msg(bb_msg_perm_denied_are_you_root); > bb_error_msg_and_die(bb_msg_memory_exhausted); > bb_error_msg_and_die(bb_msg_perm_denied_are_you_root); > bb_error_msg_and_die(bb_msg_read_error); > bb_error_msg_and_die(bb_msg_write_error); > bb_perror_msg(bb_msg_can_not_create_raw_socket); > bb_perror_msg(bb_msg_read_error); > bb_perror_msg(bb_msg_write_error); > bb_perror_msg(bb_path_mtab_file); > bb_perror_msg_and_die(bb_msg_can_not_create_raw_socket); > bb_perror_msg_and_die(bb_msg_read_error); > bb_perror_msg_and_die(bb_msg_standard_input); > bb_perror_msg_and_die(bb_msg_standard_output); > bb_perror_msg_and_die(bb_path_mtab_file); > col = fmtstr(s, 32, strsignal(st)); > printf(bbconfig_config);
There should be a way to shut up gcc in these places only instead of pessimizing the code. At least I hope so. > and some other places where it might not: > > bb_error_msg((char*)error_pkt_str); > bb_perror_msg(file); > bb_perror_msg_and_die(str); > printf(pr->fmt); > > Anyway, making -Wformat-security a default option is, arguably, for a > reason. A lot has been written about it in the passed 10 years or so. > Try: > > http://en.wikipedia.org/wiki/Format_string_attack > > which refers to this: > > http://julianor.tripod.com/bc/formatstring-1.2.pdf > > > Does it compile with CONFIG_WERROR off? > > Yes (forgot about that). > > Found another small bug. This: > > --- util-linux/mkfs_ext2.c.orig 2009-10-22 00:55:55.000000000 +0200 > +++ util-linux/mkfs_ext2.c 2009-10-24 10:45:15.370943859 +0200 > @@ -389,7 +389,7 @@ int mkfs_ext2_main(int argc UNUSED_PARAM > "%u inodes, %u blocks\n" > "%u blocks (%u%%) reserved for the super user\n" > "First data block=%u\n" > - "Maximum filesystem blocks=%u\n" > + "Maximum filesystem blocks=%lu\n" > "%u block groups\n" > "%u blocks per group, %u fragments per group\n" > "%u inodes per group" > > corrects this: > > util-linux/mkfs_ext2.c: In function 'mkfs_ext2_main': > util-linux/mkfs_ext2.c:406: error: format '%u' expects type 'unsigned int', > but argument 12 has type 'long unsigned int' Bad fix. The expression group_desc_blocks * (blocksize / sizeof(*gd)) * blocks_per_group is "long" (actually size_t) only because sizeof() is size_t. But sizeof(*gd) is very small (perhaps < 100), it's much better to cast it to unsigned than to promote entire thing to long. Fixed in git, thanks! -- vda _______________________________________________ busybox mailing list [email protected] http://lists.busybox.net/mailman/listinfo/busybox
