[comp.bugs.4bsd] Patch: Sendmail 5.61 core dumps on long lists

reggers@ria.ccs.uwo.ca (Reg Quinton) (05/31/90)

We were having trouble with sendmail core dumping when dealing with
long recipient lists (at least with 5.61 on our Mips machine). I
managed to localize the problem into deliver.c and the deliver()
procedure where the string array "tobuf" was causing the problem -- to
much data was being written into the array and this was corrupting
other structures on the stack.

The following patch fixed that problem.

*** deliver.c	Wed May 30 14:45:02 1990
--- deliver.c.orig	Wed May 30 15:35:32 1990
***************
*** 221,227 ****
  			continue;
  
  		/* avoid overflowing tobuf */
! 		if ((strlen(to->q_paddr) + strlen(tobuf) + 2) > sizeof(tobuf))
  			break;
  
  		if (tTd(10, 1))
--- 221,227 ----
  			continue;
  
  		/* avoid overflowing tobuf */
! 		if (sizeof tobuf - (strlen(to->q_paddr) + strlen(tobuf) + 2) < 0)
  			break;
  
  		if (tTd(10, 1))

Andy.Linton@comp.vuw.ac.nz (Andy Linton) (05/31/90)

In article <432@ria.ccs.uwo.ca>, reggers@ria.ccs.uwo.ca (Reg Quinton) writes:
|> We were having trouble with sendmail core dumping when dealing with
|> long recipient lists (at least with 5.61 on our Mips machine). I
|> managed to localize the problem into deliver.c and the deliver()
|> procedure where the string array "tobuf" was causing the problem -- to
|> much data was being written into the array and this was corrupting
|> other structures on the stack.
|> 
|> The following patch fixed that problem.
|> 
|> *** deliver.c	Wed May 30 14:45:02 1990
|> --- deliver.c.orig	Wed May 30 15:35:32 1990
|> ***************
|> *** 221,227 ****
|>   			continue;
|>   
|>   		/* avoid overflowing tobuf */
|> ! 		if ((strlen(to->q_paddr) + strlen(tobuf) + 2) > sizeof(tobuf))
|>   			break;
|>   
|>   		if (tTd(10, 1))
|> --- 221,227 ----
|>   			continue;
|>   
|>   		/* avoid overflowing tobuf */
|> ! 		if (sizeof tobuf - (strlen(to->q_paddr) + strlen(tobuf) + 2) < 0)
|>   			break;
|>   
|>   		if (tTd(10, 1))

I ran into this one on a a machine running HPUX 7.0. The problem there
(and I suspect on the Mips box) is the type of value (size_t) returned
by sizeof and strlen. 'size_t' is defined to be 'long' in BSD 4.3, 'int'
in SunOS and 'unsigned int' in HPUX (and ANSI C).

I checked through the sendmail code and other uses of 'sizeof' and
'strlen' are written to avoid this problem - good luck rather than judgement?

The important point is that this sort of problem will almost certainly
surface again as old code is recompiled on newer machines.

rogerk@mips.COM (Roger B.A. Klorese) (05/31/90)

In article <1990May30.221516.6077@comp.vuw.ac.nz> Andy.Linton@comp.vuw.ac.nz writes:
>In article <432@ria.ccs.uwo.ca>, reggers@ria.ccs.uwo.ca (Reg Quinton) writes:
>|> We were having trouble with sendmail core dumping when dealing with
>|> long recipient lists (at least with 5.61 on our Mips machine). 
>I ran into this one on a a machine running HPUX 7.0. The problem there
>(and I suspect on the Mips box) is the type of value (size_t) returned
>by sizeof and strlen.

This problem is fixed in the port of 5.61 which is included in Mips'
forthcoming RISC/os 4.50 release.
-- 
ROGER B.A. KLORESE      MIPS Computer Systems, Inc.      phone: +1 408 720-2939
MS 4-02    950 DeGuigne Dr.   Sunnyvale, CA  94086   voicemail: +1 408 524-7421 
rogerk@mips.COM         {ames,decwrl,pyramid}!mips!rogerk         "I'm the NLA"
"Maybe this world is another planet's hell." -- Aldous Huxley

casey@gauss.llnl.gov (Casey Leedom) (06/05/90)

| From: Andy.Linton@comp.vuw.ac.nz (Andy Linton)
| 
| The problem there (and I suspect on the Mips box) is the type of value
| (size_t) returned by sizeof and strlen. 'size_t' is defined to be 'long'
| in BSD 4.3, 'int' in SunOS and 'unsigned int' in HPUX (and ANSI C).
| 
| I checked through the sendmail code and other uses of 'sizeof' and
| 'strlen' are written to avoid this problem - good luck rather than judgement?

  No, the problem was because the test was:

	sizeof(foo) - int expression < 0

Sizeof returns unsigned which gives us:

	unsigned expression - int expression < 0

which type promoting turns into:

	unsigned expression < 0

which can never be true.  As far as I know ANSI C has not changed anything
that would affect this.

Andy.Linton@comp.vuw.ac.nz (Andy Linton) (06/06/90)

In article <60685@lll-winken.LLNL.GOV>, casey@gauss.llnl.gov (Casey
Leedom) writes:
|> 
|>   No, the problem was because the test was:
|> 
|> 	sizeof(foo) - int expression < 0
|> 
|> Sizeof returns unsigned which gives us:
|> 
|> 	unsigned expression - int expression < 0
|> 
|> which type promoting turns into:
|> 
|> 	unsigned expression < 0
|> 
|> which can never be true.  As far as I know ANSI C has not changed anything
|> that would affect this.

'sizeof' used to return an 'int' (see page 126, The C Programming
Language, Kernighan and Ritchie, 1978) and that while Harbison and
Steele (C: A Reference Manual, First Edition, Page 154, 1984)
recommended "that the result of the 'sizeof' operator be either of type
'unsigned int' or of type 'unsigned long'at the discretion of the
implementor" it wasn't standardised at that time.

In Harbison and Steele, C: A Reference Manual, Second Edition, Page 273
they say in referring to Draft Proposed ANSI C that "the result of the
'sizeof' operator may be of type 'unsigned int' or 'unsigned long'. The
type chosen by and implementation is defined as 'size_t' in the standard
header file 'stddef.h'." They also say on Page 299 that the draft
standard requires "the return type of 'strlen' to be 'size_t'". 

The point I was making was that because the type returned by 'sizeof'
and 'strlen' is now 'size_t' which is now *defined* to be 'unsigned int'
or 'unsigned long' the test fails for the reason Casey gives. Previously
the values returned by 'sizeof' and 'strlen' were implementation
dependent and the original test was written in a non-portable manner.