Re: libmd bug on -CURRENT
On Sat, 7 Sep 2002, Bruce A. Mah wrote: If memory serves me right, Bruce Evans wrote: libmd is also broken for some cases involving pipes. IIRC, this is caused by the bogus st_size checks in the same function. st_size is only valid for regular files. It's puzzling that the call to lseek(2) doesn't always return an error in these cases as well (as the manpage seems to imply). Yet, one can do md5(1) on a pipe: tomcat:bmah% cat /etc/motd | md5 9caae6eae6f9c2dfea77d6a5fae2e93c tomcat:bmah% md5 /etc/motd MD5 (/etc/motd) = 9caae6eae6f9c2dfea77d6a5fae2e93c I don't remember exactly which case(s) are broken. The above works, but cat /dev/motd | md5 /dev/stdin doesn't -- it gives the seek error. I think the open() in mdXFileChunk() gets used for the latter but not when stdin is used directly. This is with /dev/stdin not on devfs or fdescfs. Named pipes seem to work too: mkfifo p; md5 p cat /etc/motd p# same result as md5 /etc/motd The loop in the function fails to to terminate if read() returns 0, which can probably happen if the file shrinks underneath us. Maybe add a check after the read(2), so that if it returns zero, we set n = 0? It's not clear to me what result should be returned in the case of trying to compute a checksum on a file that shrinks in the middle of the computation. Writes that change the file underneath us will make a mess of the result. Hmm ... detecting changes is very easy, at least for regular files - just use fstat() and compare ctimes. Bruce To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: libmd bug on -CURRENT
On Fri, 6 Sep 2002, Bruce A. Mah wrote: If memory serves me right, Poul-Henning Kamp wrote: Good catch. I'm surprised the compiler doesn't whine. Thanks, and me too. Warnings are mostly turned off for not unimportant places like libraries since these places are too poorly written to compile without warnings. PS. Actually I'm surprised that nobody caught the problem in the past five months...this bug prevented release builds from 5-CURRENT hosts. Maybe I'm the only person crazy enough to try this. :-) This bug was caught in PR 42384. The fix in the PR is not so good. libmd is also broken for some cases involving pipes. IIRC, this is caused by the bogus st_size checks in the same function. st_size is only valid for regular files. The loop in the function fails to to terminate if read() returns 0, which can probably happen if the file shrinks underneath us. Bruce To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: libmd bug on -CURRENT
If memory serves me right, Bruce Evans wrote: On Fri, 6 Sep 2002, Bruce A. Mah wrote: If memory serves me right, Poul-Henning Kamp wrote: Good catch. I'm surprised the compiler doesn't whine. Thanks, and me too. Warnings are mostly turned off for not unimportant places like libraries since these places are too poorly written to compile without warnings. Apparently. PS. Actually I'm surprised that nobody caught the problem in the past five months...this bug prevented release builds from 5-CURRENT hosts. Maybe I'm the only person crazy enough to try this. :-) This bug was caught in PR 42384. The fix in the PR is not so good. I suppose I didn't look hard enough when I scanned through the PR database. I'll deal with the PR. libmd is also broken for some cases involving pipes. IIRC, this is caused by the bogus st_size checks in the same function. st_size is only valid for regular files. It's puzzling that the call to lseek(2) doesn't always return an error in these cases as well (as the manpage seems to imply). Yet, one can do md5(1) on a pipe: tomcat:bmah% cat /etc/motd | md5 9caae6eae6f9c2dfea77d6a5fae2e93c tomcat:bmah% md5 /etc/motd MD5 (/etc/motd) = 9caae6eae6f9c2dfea77d6a5fae2e93c The loop in the function fails to to terminate if read() returns 0, which can probably happen if the file shrinks underneath us. Maybe add a check after the read(2), so that if it returns zero, we set n = 0? It's not clear to me what result should be returned in the case of trying to compute a checksum on a file that shrinks in the middle of the computation. Thanks, Bruce. msg42775/pgp0.pgp Description: PGP signature
libmd bug on -CURRENT
I think I've found a bug in libmd on -CURRENT, in which attempting to
compute the MD5 checksum (using MD5File(3)) of a zero-length file
generates an error. A trivial way to trigger this bug (which isn't
present in 4-STABLE) is:
ref4:bmah% uname -a
FreeBSD ref4.freebsd.org 4.7-PRERELEASE FreeBSD 4.7-PRERELEASE #8: Mon Sep 2 03:20:42
PDT 2002 [EMAIL PROTECTED]:/usr/src/sys/compile/REF4 i386
ref4:bmah% ls -ls foo
0 -rw-r--r-- 1 bmah bmah 0 Sep 6 08:54 foo
ref4:bmah% md5 foo
MD5 (foo) = d41d8cd98f00b204e9800998ecf8427e
ref5:bmah% uname -a
FreeBSD ref5.freebsd.org 5.0-CURRENT FreeBSD 5.0-CURRENT #11: Mon Sep 2 03:30:53 PDT
2002 [EMAIL PROTECTED]:/usr/src/sys/i386/compile/REF5 i386
ref5:bmah% ls -ls foo
0 -rw-r--r-- 1 bmah bmah 0 Sep 6 08:54 foo
ref5:bmah% md5 foo
md5: foo: Undefined error: 0
This bug seems to have been introduced in rev. 1.14 of src/lib/libmd/
mdXhl.c; the patch below (which makes sure a variable gets initialized
before first use, even in the case of a 0-length file) seems to fix it.
Comments?
Bruce.
PS. I found this because at some point during a release build, mtree(8)
tries to compute the MD5 checksum of a zero-length file, namely
/etc/dumpdates.
Index: mdXhl.c
===
RCS file: /usr/local/cvsroot/src/lib/libmd/mdXhl.c,v
retrieving revision 1.16
diff -u -r1.16 mdXhl.c
--- mdXhl.c 25 Mar 2002 13:50:40 - 1.16
+++ mdXhl.c 6 Sep 2002 16:02:52 -
@@ -66,6 +66,7 @@
len = stbuf.st_size - ofs;
if (lseek(f, ofs, SEEK_SET) 0) return 0;
n = len;
+i = 0;
while (n 0) {
if (n sizeof(buffer))
i = read(f, buffer, sizeof(buffer));
msg42674/pgp0.pgp
Description: PGP signature
Re: libmd bug on -CURRENT
Good catch.
I'm surprised the compiler doesn't whine.
Poul-Henning
Index: mdXhl.c
===
RCS file: /usr/local/cvsroot/src/lib/libmd/mdXhl.c,v
retrieving revision 1.16
diff -u -r1.16 mdXhl.c
--- mdXhl.c 25 Mar 2002 13:50:40 - 1.16
+++ mdXhl.c 6 Sep 2002 16:02:52 -
@@ -66,6 +66,7 @@
len = stbuf.st_size - ofs;
if (lseek(f, ofs, SEEK_SET) 0) return 0;
n = len;
+i = 0;
while (n 0) {
if (n sizeof(buffer))
i = read(f, buffer, sizeof(buffer));
--
Poul-Henning Kamp | UNIX since Zilog Zeus 3.20
[EMAIL PROTECTED] | TCP/IP since RFC 956
FreeBSD committer | BSD since 4.3-tahoe
Never attribute to malice what can adequately be explained by incompetence.
To Unsubscribe: send mail to [EMAIL PROTECTED]
with unsubscribe freebsd-current in the body of the message
Re: libmd bug on -CURRENT
If memory serves me right, Poul-Henning Kamp wrote: Good catch. I'm surprised the compiler doesn't whine. Thanks, and me too. Bruce. PS. Actually I'm surprised that nobody caught the problem in the past five months...this bug prevented release builds from 5-CURRENT hosts. Maybe I'm the only person crazy enough to try this. :-) msg42694/pgp0.pgp Description: PGP signature
Re: libmd bug on -CURRENT
In message [EMAIL PROTECTED], Bruce A. Mah writes : --==_Exmh_-1692584434P Content-Type: text/plain; charset=us-ascii If memory serves me right, Poul-Henning Kamp wrote: Good catch. I'm surprised the compiler doesn't whine. Thanks, and me too. Bruce. PS. Actually I'm surprised that nobody caught the problem in the past five months...this bug prevented release builds from 5-CURRENT hosts. Maybe I'm the only person crazy enough to try this. :-) I tried that but it's probably longer than 5 months since. -- Poul-Henning Kamp | UNIX since Zilog Zeus 3.20 [EMAIL PROTECTED] | TCP/IP since RFC 956 FreeBSD committer | BSD since 4.3-tahoe Never attribute to malice what can adequately be explained by incompetence. To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
