Mailing List Archive

[Bug 770] Daemon should reinitialize len before calling accept()
------- You are receiving this mail because: -------
You are on the CC list for the bug.

http://bugs.exim.org/show_bug.cgi?id=770

Maxim Dounin <mdounin@mdounin.ru> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |mdounin@mdounin.ru




--- Comment #1 from Maxim Dounin <mdounin@mdounin.ru> 2008-11-18 12:31:38 ---
Anybody? Inlining the patch to make review easier:

--- src/daemon.c.orig Sat Aug 30 03:49:46 2008
+++ src/daemon.c Mon Sep 1 00:46:09 2008
@@ -1613,7 +1613,7 @@
struct sockaddr_in accepted;
#endif

- EXIM_SOCKLEN_T len = sizeof(accepted);
+ EXIM_SOCKLEN_T len;
pid_t pid;

/* This code is placed first in the loop, so that it gets obeyed at the
@@ -1808,6 +1808,7 @@
{
if (FD_ISSET(listen_sockets[sk], &select_listen))
{
+ len = sizeof(accepted);
accept_socket = accept(listen_sockets[sk],
(struct sockaddr *)&accepted, &len);
FD_CLR(listen_sockets[sk], &select_listen);


--
Configure bugmail: http://bugs.exim.org/userprefs.cgi?tab=email

--
## List details at http://lists.exim.org/mailman/listinfo/exim-dev Exim details at http://www.exim.org/ ##
[Bug 770] Daemon should reinitialize len before calling accept() [ In reply to ]
------- You are receiving this mail because: -------
You are on the CC list for the bug.

http://bugs.exim.org/show_bug.cgi?id=770

Ted <eximX1211@linuxwan.net> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |eximX1211@linuxwan.net




--- Comment #2 from Ted <eximX1211@linuxwan.net> 2008-11-19 22:40:01 ---
This seems to be a BSD specific bug, so perhaps wrap it in #ifdef BSD type
constructs. I don't have a BSD on me to check the code but from what you've
said, it looks like they cause changes to the value.

I looked through the accept socket calls in the Linux kernel and that variable
does not get changed, however, since it is passed as a pointer there is no way
to guarantee that it will remain unchanged on all types of system.

The tiny amount of extra effort required to calculate this for every new
connection might make it worthwhile as a unconditional change to the source.


--
Configure bugmail: http://bugs.exim.org/userprefs.cgi?tab=email

--
## List details at http://lists.exim.org/mailman/listinfo/exim-dev Exim details at http://www.exim.org/ ##
[Bug 770] Daemon should reinitialize len before calling accept() [ In reply to ]
------- You are receiving this mail because: -------
You are on the CC list for the bug.

http://bugs.exim.org/show_bug.cgi?id=770




--- Comment #3 from Kjetil Torgrim Homme <kjetilho@ifi.uio.no> 2008-11-19 23:03:29 ---
it's not a calculation, it's a static assignment, since the size is always
known during compilation. if you look at the latest Single Unix Specification,
you'll see:

address_len
Points to a socklen_t structure which on input specifies the length
of the supplied sockaddr structure, and on output specifies the length
of the stored address.

(http://www.opengroup.org/onlinepubs/009695399/functions/accept.html)

also note that the parameter is *not* declared as const, which it should be if
the function isn't allowed to change it.



--
Configure bugmail: http://bugs.exim.org/userprefs.cgi?tab=email

--
## List details at http://lists.exim.org/mailman/listinfo/exim-dev Exim details at http://www.exim.org/ ##
[Bug 770] Daemon should reinitialize len before calling accept() [ In reply to ]
------- You are receiving this mail because: -------
You are on the CC list for the bug.

http://bugs.exim.org/show_bug.cgi?id=770




--- Comment #4 from Simon Arlott <bugzilla.exim.simon@arlott.org> 2008-11-19 23:09:19 ---
The Linux man pages for accept(2) specify: "The addrlen argument is a
value-result argument: it should initially contain the size of the
structure pointed to by addr; on return it will contain the actual length (in
bytes) of the address returned."

So addrlen should always be re-initialised every time (assuming it may get
changed).


--
Configure bugmail: http://bugs.exim.org/userprefs.cgi?tab=email

--
## List details at http://lists.exim.org/mailman/listinfo/exim-dev Exim details at http://www.exim.org/ ##
[Bug 770] Daemon should reinitialize len before calling accept() [ In reply to ]
------- You are receiving this mail because: -------
You are on the CC list for the bug.

http://bugs.exim.org/show_bug.cgi?id=770

Ted <eximX1211@linuxwan.net> changed:

What |Removed |Added
----------------------------------------------------------------------------
Target Milestone|--- |Exim 4.70




--- Comment #5 from Ted <eximX1211@linuxwan.net> 2008-11-19 23:39:18 ---
So gist of all that is, len should be reset on every loop since the accept
function changes it based on the returned data. (Turns out it's
<protocol>_getname() that does it on successful accept)

Vote 1 for commit into 4.70


--
Configure bugmail: http://bugs.exim.org/userprefs.cgi?tab=email

--
## List details at http://lists.exim.org/mailman/listinfo/exim-dev Exim details at http://www.exim.org/ ##