On Tue, Aug 26, 2003 at 05:53:57PM +0200, Tomasz Kojm wrote:
> > As I don't usually do networking in C, and there were one/some style
> > complaints: Tomasz, are you accepting the patch as-is?
> Of course, I do. However I'd like to see Mark's version and after that
> we can update the CVS with the better one ;)
So... I rolled up my sleaves and got dirty.
There was a misunderstanding on my part in this discussion: I was
under the impression that bind() for a UNIX socket either worked by
default, or could be configured, similar to how bind() for an IP
socket works with SO_REUSEADDR configured. I was startled to find that
it does not. I even scanned through net/unix/af_unix.c in the Linux
kernel, and sure enough, bind() is not implemented the way I
expected. On an indirectly related topic, I believe this to be a bug
in the UNIX specification, but one that isn't likely to be fixed. :-(
The other issue that I claimed, that of a race condition in the already
proposed patch, does exist as far as I can tell. The patch appeared to
invert the bind()->stat() to be stat()->bind, which is awkward, in that
however infeasible, the stat() could show that the file does not exist,
the file could be created, and the bind() would then fail.
The patch that I enclose prefers the bind()->unlink()->bind()
approach. It does further error handling, and touches up the logged
error messages to be consistent and according to form. I don't mean to
take too much from the already proposed patch. After realizing my
first error, I used the first patch as a template to create this
one. Thanks to Thomas Lamy for getting the work-around down on paper,
and saving me from having to figure out how to add a configuration
option. I only adjusted the resulting lines to be more technically
perfect.
Cheers,
mark
--
[EMAIL PROTECTED]/[EMAIL PROTECTED]/[EMAIL PROTECTED] __________________________
. . _ ._ . . .__ . . ._. .__ . . . .__ | Neighbourhood Coder
|\/| |_| |_| |/ |_ |\/| | |_ | |/ |_ |
| | | | | \ | \ |__ . | | .|. |__ |__ | \ |__ | Ottawa, Ontario, Canada
One ring to rule them all, one ring to find them, one ring to bring them all
and in the darkness bind them...
http://mark.mielke.cc/
diff -u -r clamav-devel-orig/clamd/cfgfile.c clamav-devel/clamd/cfgfile.c
--- clamav-devel-orig/clamd/cfgfile.c 2003-08-05 23:05:51.000000000 -0400
+++ clamav-devel/clamd/cfgfile.c 2003-08-26 21:18:57.000000000 -0400
@@ -63,6 +63,7 @@
{"FollowFileSymlinks", OPT_NOARG},
{"Foreground", OPT_NOARG},
{"Debug", OPT_NOARG},
+ {"FixStaleSocket", OPT_NOARG},
{"User", OPT_STR},
{"AllowSupplementaryGroups", OPT_NOARG},
{"SelfCheck", OPT_NUM},
diff -u -r clamav-devel-orig/clamd/localserver.c clamav-devel/clamd/localserver.c
--- clamav-devel-orig/clamd/localserver.c 2003-07-29 11:48:08.000000000 -0400
+++ clamav-devel/clamd/localserver.c 2003-08-26 21:32:25.000000000 -0400
@@ -46,23 +46,41 @@
if((sockfd = socket(AF_UNIX, SOCK_STREAM, 0)) == -1) {
estr = strerror(errno);
//fprintf(stderr, "ERROR: socket() error: %s\n", estr);
- logg("!socket() error: %s\n", estr);
+ logg("!Socket allocation error: %s\n", estr);
exit(1);
}
if(bind(sockfd, (struct sockaddr *) &server, sizeof(struct sockaddr_un)) == -1) {
- if(stat(server.sun_path, &foo) != -1) {
- //fprintf(stderr, "ERROR: Socket file %s already exists. Please remove it
or use another one.\n", server.sun_path);
- logg("!Socket file %s already exists. Please remove it or use another
one.\n", server.sun_path);
+ if(errno == EADDRINUSE) {
+ if(connect(sockfd, (struct sockaddr *) &server, sizeof(struct
sockaddr_un)) >= 0) {
+ close(sockfd);
+ logg("!Socket file %s is in use by another process.\n",
server.sun_path);
+ exit(1);
+ }
+ if(cfgopt(copt, "FixStaleSocket")) {
+ logg("^Socket file %s exists. Unclean shutdown? Removing...\n",
server.sun_path);
+ if(unlink(server.sun_path) == -1) {
+ estr = strerror(errno);
+ logg("!Socket file %s could not be removed: %s\n",
server.sun_path, estr);
+ exit(1);
+ }
+ if(bind(sockfd, (struct sockaddr *) &server, sizeof(struct
sockaddr_un)) == -1) {
+ estr = strerror(errno);
+ logg("!Socket file %s could not be bound: %s (unlink tried)\n",
server.sun_path, estr);
+ exit(1);
+ }
+ } else if(stat(server.sun_path, &foo) != -1) {
+ logg("!Socket file %s exists. Either remove it, or configure a
different one.\n", server.sun_path);
+ exit(1);
+ }
+ } else {
+ estr = strerror(errno);
+ logg("!Socket file %s could not be bound: %s\n", server.sun_path, estr);
exit(1);
}
+ }
- estr = strerror(errno);
- //fprintf(stderr, "ERROR: can't bind(): %s\n", estr);
- logg("!bind() error: %s\n", estr);
- exit(1);
- } else
- logg("Unix socket file %s\n", server.sun_path);
+ logg("Unix socket file %s\n", server.sun_path);
if((cpt = cfgopt(copt, "MaxConnectionQueueLength")))
backlog = cpt->numarg;
