On Mon, 5 Jan 2009, Stephen Rothwell wrote: > Status of my local build tests will be at > http://kisskb.ellerman.id.au/linux-next . If maintainers want to give > advice about cross compilers/configs that work, we are always open to add > more builds. On m68k, you get http://kisskb.ellerman.id.au/kisskb/buildresult/65466/: | ERROR: "strlen" [drivers/md/dm-mod.ko] undefined! which triggered my attention. drivers/md/dm-sysfs.c: | static ssize_t dm_attr_name_show(struct mapped_device *md, char *buf) | { | if (dm_copy_name_and_uuid(md, buf, NULL)) | return -EIO; | | strncat(buf, "\n", DM_NAME_LEN); | return strnlen(buf, DM_NAME_LEN); | } is turned into: | .type dm_attr_name_show, @function | dm_attr_name_show: | move.l %a3,-(%sp) | move.l 12(%sp),%a3 | buf, buf | clr.l -(%sp) | | move.l %a3,-(%sp) | buf, | move.l 16(%sp),-(%sp) | md, | jbsr dm_copy_name_and_uuid | | lea (12,%sp),%sp |, | tst.l %d0 | | jeq .L23 | | move.w #-5,%a0 |, D.21217 | jbra .L25 | | .L23: | move.l %a3,-(%sp) | buf, | jbsr strlen | | addq.l #4,%sp |, | move.w #2560,(%a3,%d0.l) |,* buf | moveq #127,%d0 |, tmp43 | not.b %d0 | tmp43 | move.l %a3,%a0 | buf, sc.399 | #APP | | 1: subq.l #1,%d0 | count | jcs 2f | tst.b (%a0)+ | sc.399 | jne 1b | subq.l #1,%a0 | sc.399 | 2: | #NO_APP | sub.l %a3,%a0 | buf, D.21217 | .L25: | move.l %a0,%d0 | D.21217, <result> | move.l (%sp)+,%a3 | rts | .size dm_attr_name_show, .-dm_attr_name_show So gcc (version 4.1.2 20061115 (prerelease) (Debian 4.1.1-21) in my case) is smart and turns the strncat into a strlen() (without obeying our override of strlen() in <{linux,asm}/string.h>) followed by an _unconditional_ store. And I think that's the real bug: `strncat(buf, "\n", DM_NAME_LEN);' copies at most DM_NAME_LEN characters from source "\n", which has the known size 2, which is always smaller than DM_NAME_LEN. Hence the check is optimized away, and the "\n" is _always_ appended! Probably the intention was to limit the string in _buf_ (not the source string "\n") to DM_NAME_LEN? If yes, this may cause a buffer overflow. The same bug is present in dm_attr_uuid_show(). Fixing it will probably get rid of the undefined reference, too (I hope :-) Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe linux-next" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
