[comp.bugs.4bsd] Dbm library

meissner@xyzzy.UUCP (Michael Meissner) (01/29/88)

I remember quite awhile ago, there was a discussion of problems in the
dbm library, particularly when it has to split an index node.  Can
somebody give me a reasonably accurate summary, including if possible a
test program that shows up the bug, and/or stating that it is fixed in
BSD 4.{23}.  In terms of source licenses, Data General holds System III,
System V.{0123}, BSD 4.{23}.  Mail me responses, and if there is
interest, I will post a summary.
-- 
Michael Meissner, Data General.		Uucp: ...!mcnc!rti!xyzzy!meissner
					Arpa/Csnet:  meissner@dg-rtp.DG.COM

casey@lll-crg.llnl.gov (Casey Leedom) (01/29/88)

In article <580@xyzzy.UUCP> meissner@.UUCP (Michael Meissner) writes:
> I remember quite awhile ago, there was a discussion of problems in the
> dbm library, particularly when it has to split an index node.

  The problem is on line 499 of the 4.3BSD source for
/usr/src/lib/libc/gen/ndbm.c.  There is a comparison that involves a
sizeof which converts an expression yielding a possibly negative value
into an unsigned expression which doesn't compare right.  As it turns
out, sizeof yields an int in the VAX compiler instead of an unsigned, so
the comparison works.

  There's some question as to what the official result type of sizeof
should be since the early K&R simply said sizeof(E) was semantically an
integer constant (K&R; C Reference Manual; section 7.2; page 188).
However, later versions of K&R specified that it was an unsigned integer
constant (sorry, I don't have a copy of such a later version to
reference).

  The fix:

*** /usr/src/lib/libc/gen/ndbm.c	Sun Mar  9 19:51:28 1986
--- /tmp/xxx	Thu Jan 28 15:52:11 1988
***************
*** 496,502 ****
  	if (i2 > 0)
  		i1 = sp[i2];
  	i1 -= item.dsize + item1.dsize;
! 	if (i1 <= (i2+3) * sizeof(short))
  		return (0);
  	sp[0] += 2;
  	sp[++i2] = i1 + item1.dsize;
--- 496,502 ----
  	if (i2 > 0)
  		i1 = sp[i2];
  	i1 -= item.dsize + item1.dsize;
! 	if (i1 <= (i2+3) * (int)sizeof(short))
  		return (0);
  	sp[0] += 2;
  	sp[++i2] = i1 + item1.dsize;

Casey

casey@lll-crg.llnl.gov (Casey Leedom) (01/29/88)

  Oopps, that fix should be:

*** /usr/src/lib/libc/gen/ndbm.c	Sun Mar  9 19:51:28 1986
--- /tmp/xxx	Thu Jan 28 15:52:11 1988
***************
*** 496,502 ****
  	if (i2 > 0)
  		i1 = sp[i2];
  	i1 -= item.dsize + item1.dsize;
! 	if (i1 <= (i2+3) * sizeof(short))
  		return (0);
  	sp[0] += 2;
  	sp[++i2] = i1 + item1.dsize;
--- 496,502 ----
  	if (i2 > 0)
  		i1 = sp[i2];
  	i1 -= item.dsize + item1.dsize;
! 	if (i1 <= (i2+3) * (int)(sizeof(short)))
  		return (0);
  	sp[0] += 2;
  	sp[++i2] = i1 + item1.dsize;

Note the additional parenthesis around "sizeof(short)".

Casey

gww@beatnix.UUCP (Gary Winiger) (01/31/88)

In article <580@xyzzy.UUCP> meissner@.UUCP (Michael Meissner) writes:
> I remember quite awhile ago, there was a discussion of problems in the
> dbm library, particularly when it has to split an index node.

Some time last September, I filed the following bug report and fix.  Here
it is again for all those who need it.  The UCB VAX C compiler does not
have this problem, because it doesn't treat sizeof as an unsigned.

Gary..
{uunet,sun,lll-tis}!elxsi!gww

Subject: dbm_store fails on first attempt to write .pag file. +Fix
Index:	libc/gen/ndbm.c 4.3BSD +Fix

Description:
	dbm_store fails when the first .pag write is done.
Repeat-By:
	mkpasswd passwd
Fix:
	The comparison for .pag buffer overflow in additem fails to
	recognize overflow.  This is due to the size_t (of sizeof) being
	unsigned, thus promoting the comparison to unsigned.  The C 
	standard, in C.3.3.4, states:
	``... and its type (an unsigned integral type) is size_t.''
	Casting sizeof to int resolves this problem at ELXSI.

Gary..
{ucbvax!sun,lll-lcc!lll-tis,amdahl!altos86,bridge2}!elxsi!gww
--------- cut --------- snip --------- :.,$w diff -------------
*** /tmp/,RCSt1000709	Fri Mar 27 17:12:31 1987
--- ndbm.c	Fri Mar 27 17:12:12 1987
***************
*** 1,5 ****
--- 1,10 ----
  /*
   * $Log:	ndbm.c,v $
+  * Revision 1.2  87/03/27  17:08:45  gww
+  * Cast sizeof to int.  This comparison will fail when i1 is < 0 because the
+  * type of sizeof (according to C standard C.3.3.4) is unsigned thus causing
+  * the comparison to fail because it is promoted to unsigned.
+  * 
   * Revision 1.1  87/01/15  15:35:33  gww
   * Initial revision
   * 
***************
*** 11,17 ****
   */
  
  #if defined(LIBC_SCCS) && !defined(lint)
! static char *ERcsId = "$Header: ndbm.c,v 1.1 87/01/15 15:35:33 gww Exp $ ENIX BSD";
  static char sccsid[] = "@(#)ndbm.c	5.3 (Berkeley) 3/9/86";
  #endif LIBC_SCCS and not lint
  
--- 16,22 ----
   */
  
  #if defined(LIBC_SCCS) && !defined(lint)
! static char *ERcsId = "$Header: ndbm.c,v 1.2 87/03/27 17:08:45 gww Exp $ ENIX BSD";
  static char sccsid[] = "@(#)ndbm.c	5.3 (Berkeley) 3/9/86";
  #endif LIBC_SCCS and not lint
  
***************
*** 503,509 ****
  	if (i2 > 0)
  		i1 = sp[i2];
  	i1 -= item.dsize + item1.dsize;
! 	if (i1 <= (i2+3) * sizeof(short))
  		return (0);
  	sp[0] += 2;
  	sp[++i2] = i1 + item1.dsize;
--- 508,514 ----
  	if (i2 > 0)
  		i1 = sp[i2];
  	i1 -= item.dsize + item1.dsize;
! 	if (i1 <= (i2+3) * (int)sizeof(short))
  		return (0);
  	sp[0] += 2;
  	sp[++i2] = i1 + item1.dsize;