Subject: [PATCH] Problem with afpd unexpectedly dying (still)
From: Jonathan Paisley (jonp@chem.gla.ac.uk)
Date: Wed Mar 21 2001 - 06:07:14 EST
Hi,
I sent this message to the netatalk-devel mailing list last week, but
nobody seems to have commented on it. I also tried to post the patch on
the sourceforge project page but the file attach didn't seem to be working
and (I think) line wrapping would have destroyed the patch. Having read
some recent messages on netatalk-admins, I thought I would re-post.
Apologies for cross-posting and re-posting.
Date: Mon, 12 Mar 2001 22:47:08 +0000 (GMT)
From: Jonathan Paisley
To: netatalk-devel@lists.sourceforge.net
Subject: [PATCH] Problem with afpd unexpectedly dying
Hi,
I think I've found an possible issue with netatalk1.5pre5.
When copying some files from a netatalk share in the Finder to the local
disk, the server unexpectedly disconnects (the afpd segfaults).
I managed to track this down to the following sequence of events. This
depends on the file in question having no .AppleDouble entry. Try copying
the same file twice, without doing anything in between.
afp_openfork() is called on the data fork
afp_openfork() is called on the resource fork
-- the actual .AppleDouble file couldn't be opened, but
-- the call succeeds anyway to imitate an empty resource fork
afp_closefork() is called on the data fork returned above
** this call frees the struct adouble that's shared by the
** ofork entry. This happens because the file descriptor for the
** data and resource forks are both now set to -1.
a subsequent call to getforkparams() (e.g.) on the still open resource
fork is now referencing an invalid memory location in the stale struct
adouble.
I'm not sure what the best fix for this situation is, but I made a
temporary fix by adding a simple reference count to struct adouble which
is maintained by the code in afpd/ofork.c. I also ensure that the memory
allocated for the struct adouble in of_alloc is memset() to 0 to ensure
absolutely that ad_open initialises the structure (and the refcount)
correctly.
Is anyone else a little bit worried about relying on magic numbers for
initialisation of those struct adoubles?
Here follows the patch (I hope it hasn't got mangled!):
diff -ur netatalk-1.5pre5/etc/afpd/ofork.c
netatalk-1.5pre5.new/etc/afpd/ofork.c
--- netatalk-1.5pre5/etc/afpd/ofork.c Wed Aug 2 20:28:05 2000
+++ netatalk-1.5pre5.new/etc/afpd/ofork.c Mon Mar 12 22:16:54 2001
@@ -168,12 +168,27 @@
of = oforks[of_refnum];
/* see if we need to allocate space for the adouble struct */
- if ((of->of_ad = ad ? ad :
- calloc(1, sizeof(struct adouble))) == NULL) {
- syslog( LOG_ERR, "of_alloc: malloc: %m" );
- return NULL;
+ if (!ad) {
+
+ ad = malloc(sizeof(struct adouble));
+ if (!ad) {
+ syslog( LOG_ERR, "of_alloc: malloc: %m" );
+ return NULL;
+ }
+
+ /* initialise to zero. This is important to ensure that
+ ad_open really does reinitialise the structure. */
+ memset(ad,0,sizeof(struct adouble));
+
+ } else {
+ /* Increase the refcount on this struct adouble. This is
+ decremented again in ofork_dealloc
+ */
+ ad->ad_refcount++;
}
+ of->of_ad = ad;
+
of->of_vol = vol;
of->of_dir = dir;
@@ -252,9 +267,11 @@
oforks[ of->of_refnum ] = NULL;
free( of->of_name );
- /* free of_ad */
- if ((of->of_ad->ad_hf.adf_fd == -1) &&
- (of->of_ad->ad_df.adf_fd == -1)) {
+
+ /* decrease refcount */
+ of->of_ad->ad_refcount--;
+
+ if (of->of_ad->ad_refcount<=0) {
free( of->of_ad);
} else {/* someone's still using it. just free this user's locks */
ad_unlock(of->of_ad, of->of_refnum);
diff -ur netatalk-1.5pre5/include/atalk/adouble.h
netatalk-1.5pre5.new/include/atalk/adouble.h
--- netatalk-1.5pre5/include/atalk/adouble.h Wed Oct 4 16:21:41 2000
+++ netatalk-1.5pre5.new/include/atalk/adouble.h Mon Mar 12
22:17:55 2001
@@ -181,6 +181,7 @@
struct ad_entry ad_eid[ ADEID_MAX ];
struct ad_fd ad_df, ad_hf;
int ad_flags, ad_inited;
+ int ad_refcount; /* used in afpd/ofork.c */
#ifdef USE_MMAPPED_HEADERS
char *ad_data;
#else
diff -ur netatalk-1.5pre5/libatalk/adouble/ad_open.c
netatalk-1.5pre5.new/libatalk/adouble/ad_open.c
--- netatalk-1.5pre5/libatalk/adouble/ad_open.c Thu Mar 1 18:11:33 2001
+++ netatalk-1.5pre5.new/libatalk/adouble/ad_open.c Mon Mar 12
20:44:30 2001
@@ -600,6 +600,8 @@
ad->ad_data = MAP_FAILED;
#endif
ad->ad_inited = AD_INITED;
+
+ ad->ad_refcount = 1;
}
if (adflags & ADFLAGS_DF) {
-- Jonathan Paisley jonp@chem.gla.ac.uk
This archive was generated by hypermail 2b28 : Sun Oct 14 2001 - 03:04:35 EDT