[EXIM] Memory-management bug (2.02, Debian -3)

Top Page
Delete this message
Reply to this message
Author: Ian Jackson
Date:  
To: exim-users
Subject: [EXIM] Memory-management bug (2.02, Debian -3)
In verify.c, the functions verify_setup_hostlist and
verify_setup_netlist are used to cache the in-memory forms of host and
net lists; they are called from various places, mainly in smtp_in.c,
always with a pointer to static global variable for `anchor' (the
parameter indicating which in-memory list to store).

Unfortunately, when building the data structures to put into these
variables, they use the `store_get' function which returns memory to
the pool after a message has been processed. This means that the
memory for these data structures can get reused later !

The solution is to use the permanent allocator, store_malloc. I also
had to add a `copy_string_malloc' function to string.c.

Please see the patch below. It is quite possible that noone else has
noticed this problem yet, as I'm experimenting with another patch to
change the behaviour when exim is invoked with -bs -oMa when stdin is
not a TCP socket. More about this later.

I've verified that the patch below fixes the problem for me.

On my system with an Exim with another patch the problem can be
reproduced by becoming `mail' and running
exim -bs -oem -oMa 127.0.0.1 -oMr smtp -oMs localhost -oMt ian <t
however a standard exim doesn't do this because of some unexpected
handling of the -oMa option. Please see my forthcoming message for
details of the purpose patch that I'm using. For the moment I've
included that patch too for completeness.

A copy of the input file `t' and my exim.conf are below. Note that
I've manually split the diff hunks below into the two categories; I
may have got this wrong, in which case I apologise.

Thanks,
Ian.


FIRST PATCH - MEMORY MANAGEMENT BUGFIX
Believed suitable for general distribution; not already reported.
In the presence of the 2nd and 3rd patches this patch makes the
difference between a working and non-working Exim.

--- ./src/verify.c~    Mon Aug  3 11:27:37 1998
+++ ./src/verify.c    Thu Sep 10 00:56:59 1998
@@ -572,14 +572,14 @@
      s != NULL;
      s = string_nextinlist(&list, ':', buffer, sizeof(buffer)))
   {
-  host_item *h = store_get(sizeof(host_item));
+  host_item *h = store_malloc(sizeof(host_item));
   h->next = NULL;
   *anchor = h;
   anchor = &(h->next);


/* Make a permanent copy of the string. */

- s = string_copy(s);
+ s = string_copy_malloc(s);

/* If there's an '@' in the string that is not the last character, it starts
with an ident string; carve that off. */
@@ -595,7 +595,7 @@
/* If the host name string is precisely "@", then substitute the primary
host name. */

- h->name = (strcmp(s, "@") == 0)? string_copy(primary_hostname) : s;
+ h->name = (strcmp(s, "@") == 0)? string_copy_malloc(primary_hostname) : s;
h->compiled_name = NULL;
h->address = NULL;

@@ -782,7 +782,7 @@
{
int i;
char *error;
- ip_net_item *n = store_get(sizeof(ip_net_item));
+ ip_net_item *n = store_malloc(sizeof(ip_net_item));

n->next = NULL;
*anchor = n;
@@ -793,7 +793,7 @@

   if (*s == '/')
     {
-    n->filename = string_copy(s);
+    n->filename = string_copy_malloc(s);
     continue;
     }


--- ./src/string.c~    Mon Aug  3 11:27:37 1998
+++ ./src/string.c    Thu Sep 10 00:57:01 1998
@@ -388,6 +388,25 @@



 /*************************************************
+*    Copy and save string using store_malloc     *
+*************************************************/
+
+/*
+Argument: string to copy
+Returns:  copy of string in new store
+*/
+
+char *
+string_copy_malloc(char *s)
+{
+char *ss = store_malloc((int)strlen(s) + 1);
+strcpy(ss, s);
+return ss;
+}
+
+
+
+/*************************************************
 *            Copy and save string                *
 *************************************************/


--- ./src/functions.h~    Mon Aug  3 11:27:34 1998
+++ ./src/functions.h    Thu Sep 10 00:57:01 1998
@@ -178,6 +178,7 @@
 extern char *string_copylc(char *);
 extern char *string_copyn(char *, int);
 extern char *string_copynlc(char *, int);
+extern char *string_copy_malloc(char *);
 extern BOOL  string_format(char *, int, char *, ...);
 extern char *string_format_size(int, char *);
 extern int   string_interpret_escape(char **);




SECOND PATCH - DSN RECIPIENTS_LIST BUGFIX
Believed suitable for general distribution, but _already reported_.
I don't think it's relevant. I include it only because I reproduced
the bug and then solved it with an Exim which has had this applied.

--- ./src/accept.c~    Mon Aug  3 11:27:33 1998
+++ ./src/accept.c    Thu Sep 10 00:30:10 1998
@@ -270,11 +270,12 @@
   }
 recipients_list[recipients_count].address = receiver;
 recipients_list[recipients_count].flags = flags;
-recipients_list[recipients_count++].pno = pno;


#ifdef SUPPORT_DSN
recipients_list[recipients_count].orcpt = orcpt;
#endif
+
+recipients_list[recipients_count++].pno = pno;
}




THIRD PATCH - FUNCTIONALITY ENHANCEMENT FOR -oMa
Please see my forthcoming message for the status of this patch.
Without it Exim behaves fine in my test case. I include it so that
you can reproduce the bug.

--- ./src/exim.c~    Mon Aug  3 11:27:34 1998
+++ ./src/exim.c    Wed Sep  9 23:45:13 1998
@@ -1659,7 +1659,7 @@
 get it now, because some OS require the first call to os_getloadavg() to
 be done as root. What a pain. */


-if ((is_inetd && smtp_load_reserve >= 0) ||
+if (((is_inetd || sender_host_address) && smtp_load_reserve >= 0) ||
     (queue_only_load >= 0 &&
       (smtp_input || extract_recipients ||
         (recipients_arg < argc && !checking))))
@@ -2463,6 +2463,13 @@
   verify_get_ident(0);
   host_build_sender_fullhost();
   set_process_info("handling incoming connection from %s via inetd",
+    sender_fullhost);
+  }
+
+if (!is_inetd && sender_host_address)
+  {
+  host_build_sender_fullhost();
+  set_process_info("handling incoming connection from %s with -oMa",
     sender_fullhost);
   }


--- ./src/smtp_in.c~    Mon Aug  3 11:27:37 1998
+++ ./src/smtp_in.c    Wed Sep  9 23:52:48 1998
@@ -908,11 +908,14 @@
     if (getsockopt(fileno(smtp_out), IPPROTO_IP, IP_OPTIONS, (char *)(ipopt),
           &optlen) < 0)
       {
-      log_write(0, LOG_MAIN, "getsockopt() failed from %s: %s",
-        host_and_ident(), strerror(errno));
-      DEBUG(3) debug_printf("451 SMTP service not available\n");
-      fprintf(smtp_out, "451 SMTP service not available\r\n");
-      return FALSE;
+      if (errno != ENOTSOCK)
+        {
+        log_write(0, LOG_MAIN, "getsockopt() failed from %s: %s",
+          host_and_ident(), strerror(errno));
+        DEBUG(3) debug_printf("451 SMTP service not available\n");
+        fprintf(smtp_out, "451 SMTP service not available\r\n");
+        return FALSE;
+        }
       }


     else if (optlen > 0)



TEST INPUT FILE `t'

helo anarres.greenend.org.uk
mail from:<discard-all@???>
rcpt to:<ian@???>
data
Subject: normal test many

.
mail from:<discard-all@???>
rcpt to:<ian@???>
data
Subject: normal test many

.
mail from:<discard-all@???>
rcpt to:<ian@???>
data
Subject: normal test many

.
mail from:<discard-all@???>
rcpt to:<ian@???>
data
Subject: normal test many

.


EXIM.CONF

# This is the main exim configuration file.
# It was originally generated by `eximconfig', part of the exim package
# distributed with Debian, but it may edited by the mail system administrator.
# This file originally generated by eximconfig at Mon Dec 8 00:38:39 GMT 1997
# See exim info section for details of the things that can be configured here.
# General configuration here, such as local domains

LOCAL_HOSTS=anarres.greenend.org.uk:localhost

#local_interfaces=127.0.0.1

qualify_domain=davenant.greenend.org.uk

local_domains=
local_domains_include_host
local_domains_include_host_literals

accept_8bitmime
delivery_date_remove
envelope_to_remove
return_path_remove
receiver_verify_except_hosts=LOCAL_HOSTS
smtp_verify
receiver_verify
sender_verify
sender_verify_log_details
sender_verify_reject
percent_hack_domains=
security=setuid+seteuid
trusted_users=majordom:mail:ian
trusted_groups=mail:daemon:news:majordom:root
primary_hostname=anarres.greenend.org.uk
receiver_unqualified_hosts=LOCAL_HOSTS
sender_unqualified_hosts=LOCAL_HOSTS
sender_host_accept_relay=LOCAL_HOSTS
relay_domains=davenant.greenend.org.uk
log_smtp_confirmation

gecos_pattern = ^([^,:]*)
gecos_name = $1

received_header_text = "Received: \
          ${if def:sender_fullhost {from ${sender_fullhost} \
          ${if def:sender_ident {(${sender_ident})}}\n\t}\
          {${if def:sender_ident {from ${sender_ident} }}}}\
          by ${primary_hostname} \
          ${if def:received_protocol {with ${received_protocol}}} \
          (Exim ${version_number} #${compile_number})\n\t\
          id ${message_id} (Debian)"
end


######################################################################
#                      TRANPORTS CONFIGURATION                       #
######################################################################


#local_delivery:
# driver = appendfile;
# group = mail
# mode = 0660
# file = /var/spool/mail/${local_part}

#address_pipe:
# driver = pipe;

#address_file:
# driver = appendfile

#address_reply:
# driver = autoreply


# General configuration for SMTP delivery
smtp:
driver = smtp;


end

######################################################################
#                      DIRECTORS CONFIGURATION                       #
######################################################################



#real_local:
# prefix = real-,
# driver = localuser,
# transport = local_delivery;


#system_aliases:
# driver = aliasfile;
# file = /etc/aliases,
# search_type = lsearch
# user = list
# Uncomment the above line if you are running smartlist

#userforward:
# no_verify,
# driver = forwardfile;
# file = .forward,
## filter


smart:
driver = smartuser;
new_address = ${local_part}@???


end

######################################################################
#                      ROUTERS CONFIGURATION                         #
######################################################################



smarthost:
driver = domainlist,
transport = smtp;
route_list = "* davenant.greenend.org.uk bydns_mx"

end

######################################################################
#                      RETRY CONFIGURATION                           #
######################################################################


# Domain               Error       Retries
# ------               -----       -------


*                      *           F,2h,15m; G,16h,2h,1.5; F,4d,8h


end

######################################################################
#                      REWRITE CONFIGURATION                         #
######################################################################


# These rewriters make sure the mail messages appear to have originated
# from the real mail-reading host.

#^(root|postmaster|mailer-daemon)@davenant.greenend.org.uk ${1}@??? Ffr
#*@davenant.greenend.org.uk ${1}@??? Ffr
#*@in.limbo ian@??? Ffr


# End of Exim configuration file

--
*** Exim information can be found at http://www.exim.org/ ***