[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PATCH] nfs audit
|
From: |
Neal H Walfield |
|
Subject: |
[PATCH] nfs audit |
|
Date: |
Wed, 27 Dec 2000 00:27:59 -0500 |
|
User-agent: |
Mutt/1.2.5i |
I have been auditing the nfs client code. Here is a patch that corrects
several errors. There are several places where I was not sure if what
was happening was correct; they are marked with XXX. I have tested this
patch through both general usage and stress testing it; as far as I can
tell, it has introduced no new errors.
-Neal
--
Neal H Walfield
University of Massachusetts at Lowell
neal@walfield.org or neal@cs.uml.edu
Index: nfs/ChangeLog
===================================================================
RCS file: /cvs/hurd/nfs/ChangeLog,v
retrieving revision 1.39
diff -u -r1.39 ChangeLog
--- nfs/ChangeLog 2000/11/26 22:37:46 1.39
+++ nfs/ChangeLog 2000/12/27 04:12:21
@@ -1,3 +1,41 @@
+2000-12-26 Neal H Walfield <neal@cs.uml.edu>
+
+ * cache.c: Change cache/hash table size to 509, a prime. Use
+ memcpy/memcmp not bcopy/bcmp. Verify return value from malloc and
+ check the result of rpc transaction _and_ do not act if failed.
+ * main.c: Correct the wording of the help messages. Do not
+ bother initializing global variable to 0. Use memcpy/memcmp not
+ bcopy/bcmp. Verify return value from malloc and check the result
+ of rpc transaction _and_ do not act if failed.
+ * mount.c: Check return values of initialize_rpc. Use
+ memcpy/memcmp not bcopy/bcmp. Verify return value from malloc and
+ strdup. Correct comments.
+ (mount_root): Check result of connect. Handle errors
+ consistently. Reverse loops that are if (! c) {} else when
+ appropriate.
+ * mount.h: Protect header with #ifdef.
+ * name-cache.c: Correct dangerous NPARTIALS macro. Use
+ memcpy/memcmp not bcopy/bcmp.
+ (find_cache): Use PARTIAL_THRESH, not the constant.
+ * nfs-spec.h: Protect header with #ifdef.
+ * nfs.c: Use memcpy/memcmp not bcopy/bcmp.
+ * nfs.h: Likewise.
+ * ops.c (netfs_attempt_mkdir): Check return values of initialize_rpc.
+ Use memcpy/memcmp not bcopy/bcmp. Verify return value from malloc and
+ check the result of rpc transaction _and_ do not act if failed.
+ (netfs_attempt_link): Unlock the directory before the rpc transaction.
+ Check the result of rpc transaction _and_ do not act if failed.
+ * pager.c: Remove, we do not use it.
+ * rpc.c: Use memcpy/memcmp not bcopy/bcmp. Verify return value from
+ malloc and check the result of rpc transaction _and_ do not act if
+ failed.
+ (initialize_rpc): Use AUTH_NONE, not the depreciated
+ AUTH_NULL. Return sane values on failure.
+ (generate_xid): Make inline.
+ (link_rpc): New function. Complements unlink_rpc.
+ (conduct_rpc): Use link_rpc.
+ (rpc_receive_thread): Reroll to a single loop.
+
2000-11-26 Marcus Brinkmann <marcus@gnu.org>
* ops.c (netfs_attempt_mkdir): Add casts -1 -> (struct iouser *) -1
Index: nfs/cache.c
===================================================================
RCS file: /cvs/hurd/nfs/cache.c,v
retrieving revision 1.14
diff -u -r1.14 cache.c
--- nfs/cache.c 1997/08/07 18:30:36 1.14
+++ nfs/cache.c 2000/12/27 04:12:22
@@ -23,8 +23,11 @@
#include <string.h>
#include <netinet/in.h>
-/* Hash table containing all the nodes currently active. */
-#define CACHESIZE 512
+/* Hash table containing all the nodes currently active.
+ XXX Was 512, however, a prime is much nice for the hash
+ function. 509 is nice as not only is it prime, it keeps
+ the array within a page or two */
+#define CACHESIZE 509
static struct node *nodehash [CACHESIZE];
/* Compute and return a hash key for NFS file handle DATA of LEN bytes. */
@@ -44,7 +47,8 @@
/* Lookup the file handle P (length LEN) in the hash table. If it is
not present, initialize a new node structure and insert it into the
hash table. Whichever course, a new reference is generated and the
- node is returned in *NPP. */
+ node is returned in *NPP; the lock on the node, (*NPP)->LOCK, is
+ held. */
void
lookup_fhandle (void *p, size_t len, struct node **npp)
{
@@ -58,7 +62,7 @@
for (np = nodehash[h]; np; np = np->nn->hnext)
{
if (np->nn->handle.size != len
- || bcmp (np->nn->handle.data, p, len) != 0)
+ || memcmp (np->nn->handle.data, p, len) != 0)
continue;
np->references++;
@@ -68,9 +72,12 @@
return;
}
+ /* Could not find it */
nn = malloc (sizeof (struct netnode));
+ assert (nn);
+
nn->handle.size = len;
- bcopy (p, nn->handle.data, len);
+ memcpy (nn->handle.data, p, len);
nn->stat_updated = 0;
nn->dtrans = NOT_POSSIBLE;
nn->dead_dir = 0;
@@ -112,8 +119,9 @@
};
/* Called by libnetfs when node NP has no more references. (See
- <hurd/libnetfs.h> for details. Just clear local state and remove
- from the hash table. */
+ <hurd/libnetfs.h> for details. Just clear its local state and
+ remove it from the hash table. Called and expected to leave with
+ NETFS_NODE_REFCNT_LOCK held. */
void
netfs_node_norefs (struct node *np)
{
@@ -122,6 +130,7 @@
struct fnd *args;
args = malloc (sizeof (struct fnd));
+ assert (args);
np->references++;
spin_unlock (&netfs_node_refcnt_lock);
@@ -153,7 +162,7 @@
/* Change the file handle used for node NP to be the handle at P.
Make sure the hash table stays up to date. Return the address
- after the hnadle. */
+ after the handle. The lock on the node should be held. */
int *
recache_handle (int *p, struct node *np)
{
@@ -165,14 +174,17 @@
else
len = ntohl (*p++);
+ /* Unlink it */
spin_lock (&netfs_node_refcnt_lock);
*np->nn->hprevp = np->nn->hnext;
if (np->nn->hnext)
np->nn->hnext->nn->hprevp = np->nn->hprevp;
+ /* Change the name */
np->nn->handle.size = len;
- bcopy (p, np->nn->handle.data, len);
+ memcpy (np->nn->handle.data, p, len);
+ /* Reinsert it */
h = hash (p, len);
np->nn->hnext = nodehash[h];
if (np->nn->hnext)
@@ -182,5 +194,4 @@
spin_unlock (&netfs_node_refcnt_lock);
return p + len / sizeof (int);
}
-
Index: nfs/main.c
===================================================================
RCS file: /cvs/hurd/nfs/main.c,v
retrieving revision 1.28
diff -u -r1.28 main.c
--- nfs/main.c 1997/08/07 18:30:39 1.28
+++ nfs/main.c 2000/12/27 04:12:24
@@ -34,13 +34,13 @@
extern char *localhost ();
/* Default number of times to retry RPCs when mounted soft. */
-#define DEFAULT_SOFT_RETRIES 3
+#define DEFAULT_SOFT_RETRIES 3
/* Default number of seconds to timeout cached stat information. */
#define DEFAULT_STAT_TIMEOUT 3
/* Default number of seconds to timeout cached file contents. */
-#define DEFAULT_CACHE_TIMEOUT 3
+#define DEFAULT_CACHE_TIMEOUT 3
/* Default number of seconds to timeout cache positive dir hits. */
#define DEFAULT_NAME_CACHE_TIMEOUT 3
@@ -49,10 +49,10 @@
#define DEFAULT_NAME_CACHE_NEG_TIMEOUT 3
/* Default maximum number of bytes to read at once. */
-#define DEFAULT_READ_SIZE 8192
+#define DEFAULT_READ_SIZE 8192
/* Default maximum number of bytes to write at once. */
-#define DEFAULT_WRITE_SIZE 8192
+#define DEFAULT_WRITE_SIZE 8192
/* Number of seconds to timeout cached stat information. */
@@ -114,8 +114,8 @@
{
{0,0,0,0,0,1},
{"soft", OPT_SOFT, "RETRIES", OPTION_ARG_OPTIONAL,
- "File system requests will eventually fail, after RETRIES tries if"
- " specified, otherwise " _D(SOFT_RETRIES)},
+ "File system requests will eventually fail, after RETRIES tries"
+ " (default " _D(SOFT_RETRIES) ")" },
{"hard", OPT_HARD, 0, 0,
"Retry file systems requests until they succeed"},
@@ -134,10 +134,10 @@
"Timeout for cached file data (default " _D(CACHE_TIMEOUT) ")"},
{"name-cache-timeout", OPT_NCACHE_TO, "SEC", 0,
"Timeout for positive directory cache entries (default "
- _D(NAME_CACHE_TIMEOUT) ")"},
+ _D(NAME_CACHE_TIMEOUT) ")"},
{"name-cache-neg-timeout", OPT_NCACHE_NEG_TO, "SEC", 0,
"Timeout for negative directory cache entires (default "
- _D(NAME_CACHE_NEG_TIMEOUT) ")"},
+ _D(NAME_CACHE_NEG_TIMEOUT) ")"},
{"init-transmit-timeout", OPT_INIT_TR_TO,"SEC", 0},
{"max-transmit-timeout", OPT_MAX_TR_TO, "SEC", 0},
@@ -197,7 +197,7 @@
static char *args_doc = "REMOTE_FS [HOST]";
static char *doc = "Hurd nfs translator"
"\vIf HOST is not specified, an attempt is made to extract"
-" it from REMOTE_FS, using either the `HOST:FS' or `FS@HOST' notations.";
+" it from REMOTE_FS using either the `HOST:FS' or `FS@HOST' notations.";
static const struct argp_child
runtime_argp_children[] = { {&netfs_std_runtime_argp}, {0} };
@@ -205,12 +205,12 @@
runtime_argp = { common_options, parse_common_opt, 0, 0,
runtime_argp_children };
-/* Use by netfs_set_options to handle runtime option parsing. */
+/* Used by netfs_set_options to handle runtime option parsing. */
struct argp *netfs_runtime_argp = &runtime_argp;
/* Where to find the remote filesystem. */
-static char *remote_fs = 0;
-static char *host = 0;
+static char *remote_fs; /* = 0; */
+static char *host; /* = 0; */
/* Return an argz string describing the current options. Fill *ARGZ
with a pointer to newly malloced storage holding the list and *LEN
@@ -272,6 +272,8 @@
char *sep;
spec = strdup (spec); /* So we can trash it. */
+ if (! spec)
+ return NULL;
sep = index (spec, ':');
if (sep)
Index: nfs/mount.c
===================================================================
RCS file: /cvs/hurd/nfs/mount.c,v
retrieving revision 1.22
diff -u -r1.22 mount.c
--- nfs/mount.c 1998/07/20 06:55:44 1.22
+++ nfs/mount.c 2000/12/27 04:12:25
@@ -87,13 +87,13 @@
}
/* Using the mount protocol, lookup NAME at host HOST.
- Return a node for it or null for an error. */
+ Return a node for it or null for an error. If an
+ error occurs, a message is automatically sent to stderr. */
struct node *
mount_root (char *name, char *host)
{
struct sockaddr_in addr;
struct hostent *h;
- struct servent *s;
int *p;
void *rpcbuf;
int port;
@@ -103,6 +103,14 @@
/* Lookup the portmapper port number */
if (pmap_service_name)
{
+ struct servent *s;
+
+ /* XXX This will always fail! pmap_service_name will always be "sunrpc"
+ What should pmap_service_name really be? By definition the second
+ argument is either "tcp" or "udp" Thus, is this backwards
+ (as service_name suggests)? If so, should it read:
+ s = getservbyname (pmap_service_name, "udp");
+ or is there something I am missing here? */
s = getservbyname ("sunrpc", pmap_service_name);
if (s)
pmapport = s->s_port;
@@ -121,17 +129,29 @@
}
addr.sin_family = h->h_addrtype;
- bcopy (h->h_addr_list[0], &addr.sin_addr, h->h_length);
+ memcpy (&addr.sin_addr, h->h_addr_list[0], h->h_length);
addr.sin_port = pmapport;
-
- connect (main_udp_socket,
- (struct sockaddr *)&addr, sizeof (struct sockaddr_in));
- if (!mount_port_override)
+ if (mount_port_override)
+ addr.sin_port = htons (mount_port);
+ else
{
/* Formulate and send a PMAPPROC_GETPORT request
to lookup the mount program on the server. */
+ if (connect (main_udp_socket, (struct sockaddr *)&addr,
+ sizeof (struct sockaddr_in)) == -1)
+ {
+ perror ("server mount program");
+ return 0;
+ }
+
p = pmap_initialize_rpc (PMAPPROC_GETPORT, &rpcbuf);
+ if (! p)
+ {
+ perror ("creating rpc packet");
+ return 0;
+ }
+
*p++ = htonl (MOUNTPROG);
*p++ = htonl (MOUNTVERS);
*p++ = htonl (IPPROTO_UDP);
@@ -146,38 +166,43 @@
addr.sin_port = htons (mount_port);
else
{
- free (rpcbuf);
perror ("portmap of mount");
- return 0;
+ goto error_with_rpcbuf;
}
free (rpcbuf);
}
- else
- addr.sin_port = htons (mount_port);
-
- /* Now talking to the mount program, fetch the file handle
+ /* Now, talking to the mount program, fetch a file handle
for the root. */
- connect (main_udp_socket,
- (struct sockaddr *) &addr, sizeof (struct sockaddr_in));
+ if (connect (main_udp_socket, (struct sockaddr *) &addr,
+ sizeof (struct sockaddr_in)) == -1)
+ {
+ perror ("connect");
+ goto error_with_rpcbuf;
+ }
+
p = mount_initialize_rpc (MOUNTPROC_MNT, &rpcbuf);
+ if (! p)
+ {
+ perror ("rpc");
+ goto error_with_rpcbuf;
+ }
+
p = xdr_encode_string (p, name);
errno = conduct_rpc (&rpcbuf, &p);
if (errno)
{
- free (rpcbuf);
perror (name);
- return 0;
+ goto error_with_rpcbuf;
}
/* XXX Protocol spec says this should be a "unix error code"; we'll
- pretend that an NFS error code is what's meant, the numbers match
+ pretend that an NFS error code is what's meant; the numbers match
anyhow. */
errno = nfs_error_trans (htonl (*p++));
if (errno)
{
- free (rpcbuf);
perror (name);
- return 0;
+ goto error_with_rpcbuf;
}
/* Create the node for root */
@@ -185,13 +210,25 @@
free (rpcbuf);
mutex_unlock (&np->lock);
- if (!nfs_port_override)
+ if (nfs_port_override)
+ port = nfs_port;
+ else
{
- /* Now send another PMAPPROC_GETPORT request to lookup the nfs server. */
+ /* Send another PMAPPROC_GETPORT request to lookup the nfs server. */
addr.sin_port = pmapport;
- connect (main_udp_socket,
- (struct sockaddr *) &addr, sizeof (struct sockaddr_in));
+ if (connect (main_udp_socket, (struct sockaddr *) &addr,
+ sizeof (struct sockaddr_in)) == -1)
+ {
+ perror ("connect");
+ return 0;
+ }
+
p = pmap_initialize_rpc (PMAPPROC_GETPORT, &rpcbuf);
+ if (! p)
+ {
+ perror ("rpc");
+ goto error_with_rpcbuf;
+ }
*p++ = htonl (NFS_PROGRAM);
*p++ = htonl (NFS_VERSION);
*p++ = htonl (IPPROTO_UDP);
@@ -203,18 +240,24 @@
port = nfs_port;
else
{
- free (rpcbuf);
perror ("portmap of nfs server");
- return 0;
+ goto error_with_rpcbuf;
}
free (rpcbuf);
}
- else
- port = nfs_port;
addr.sin_port = htons (port);
- connect (main_udp_socket,
- (struct sockaddr *) &addr, sizeof (struct sockaddr_in));
+ if (connect (main_udp_socket, (struct sockaddr *) &addr,
+ sizeof (struct sockaddr_in)) == -1)
+ {
+ perror ("connect");
+ return 0;
+ }
return np;
+
+error_with_rpcbuf:
+ free (rpcbuf);
+
+ return 0;
}
Index: nfs/mount.h
===================================================================
RCS file: /cvs/hurd/nfs/mount.h,v
retrieving revision 1.4
diff -u -r1.4 mount.h
--- nfs/mount.h 1996/09/04 12:46:32 1.4
+++ nfs/mount.h 2000/12/27 04:12:25
@@ -20,6 +20,9 @@
/* These constants define the RPC mount protocol; see RFC 1094. */
+#ifndef NFS_MOUNT_H
+#define NFS_MOUNT_H
+
#define MOUNTPROG 100005
#define MOUNTVERS 1
@@ -33,3 +36,5 @@
#define MOUNTPROC_UMNT 3
#define MOUNTPROC_UMNTALL 4
#define MOUNTPROC_EXPORT 5
+
+#endif /* NFS_MOUNT_H */
Index: nfs/name-cache.c
===================================================================
RCS file: /cvs/hurd/nfs/name-cache.c,v
retrieving revision 1.2
diff -u -r1.2 name-cache.c
--- nfs/name-cache.c 1997/08/07 18:30:42 1.2
+++ nfs/name-cache.c 2000/12/27 04:12:26
@@ -24,7 +24,7 @@
#include <cacheq.h>
-/* Maximum number of names to cache at once */
+/* Maximum number of names to cache at any given time */
#define MAXCACHE 200
/* Maximum length of file name we bother caching */
@@ -73,7 +73,7 @@
} statistics;
#define PARTIAL_THRESH 100
-#define NPARTIALS MAXCACHE / PARTIAL_THRESH
+#define NPARTIALS (MAXCACHE / PARTIAL_THRESH)
struct stats partial_stats [NPARTIALS];
@@ -93,19 +93,19 @@
if (c->name_len == name_len
&& c->dir_cache_len == len
&& c->name[0] == name[0]
- && bcmp (c->dir_cache_fh, dir, len) == 0
+ && memcmp (c->dir_cache_fh, dir, len) == 0
&& strcmp (c->name, name) == 0)
{
- c->stati = i / 100;
+ c->stati = i / PARTIAL_THRESH;
return c;
}
return 0;
}
-/* Node NP has just been found in DIR with NAME. If NP is null, that
- means that this name has been confirmed as absent in the directory.
- DIR is the fhandle of the directory; its length is LEN. */
+/* Node NP has just been found in DIR with NAME. If NP is null, this
+ name has been confirmed as absent in the directory. DIR is the
+ fhandle of the directory and LEN is its length. */
void
enter_lookup_cache (char *dir, size_t len, struct node *np, char *name)
{
@@ -127,7 +127,7 @@
c = find_cache (dir, len, name, name_len) ?: lookup_cache.lru;
/* Fill C with the new entry. */
- bcopy (dir, c->dir_cache_fh, len);
+ memcpy (c->dir_cache_fh, dir, len);
c->dir_cache_len = len;
if (c->np)
netfs_nrele (c->np);
@@ -158,7 +158,8 @@
if (c->name_len == namelen
&& c->dir_cache_len == dp->nn->handle.size
- && bcmp (c->dir_cache_fh, dp->nn->handle.data, c->dir_cache_len) == 0
+ && memcmp (c->dir_cache_fh, dp->nn->handle.data,
+ c->dir_cache_len) == 0
&& strcmp (c->name, name) == 0)
{
if (c->np)
@@ -237,11 +238,12 @@
-/* Scan the cache looking for NAME inside DIR. If we don't know
- anything entry at all, then return 0. If the entry is confirmed to
- not exist, then return -1. Otherwise, return NP for the entry, with
- a newly allocated reference. For any return value but 0, unlock
- DP before returning. */
+/* Scan the cache looking for NAME inside DIR. If we know nothing
+ about the entry, then return 0. If the entry is confirmed to not
+ exist, then return -1. Otherwise, return NP for the entry, with
+ a newly allocated reference. For all return values other than 0,
+ unlock DIR->LOCK before returning. For positive hits, lock the
+ returned node. */
struct node *
check_lookup_cache (struct node *dir, char *name)
{
Index: nfs/nfs-spec.h
===================================================================
RCS file: /cvs/hurd/nfs/nfs-spec.h,v
retrieving revision 1.1
diff -u -r1.1 nfs-spec.h
--- nfs/nfs-spec.h 1996/09/04 12:46:34 1.1
+++ nfs/nfs-spec.h 2000/12/27 04:12:27
@@ -1,3 +1,6 @@
+#ifndef NFS_NFS_SPEC_H
+#define NFS_NFS_SPEC_H
+
#define NFS_PORT 2049
#define NFS_MAXDATA 8192
#define NFS_MAXPATHLEN 1024
@@ -162,3 +165,4 @@
#define NFS3PROC_PATHCONF 20
#define NFS3PROC_COMMIT 21
+#endif /* NFS_NFS_SPEC_H */
Index: nfs/nfs.c
===================================================================
RCS file: /cvs/hurd/nfs/nfs.c,v
retrieving revision 1.25
diff -u -r1.25 nfs.c
--- nfs/nfs.c 1999/01/31 23:50:34 1.25
+++ nfs/nfs.c 2000/12/27 04:12:31
@@ -154,7 +154,7 @@
p[nints] = 0;
*p++ = htonl (len);
- bcopy (data, p, len);
+ memcpy (p, data, len);
return p + nints;
}
@@ -379,6 +379,7 @@
size_t len;
len = protocol_version == 2 ? NFS2_FHSIZE : ntohl (*p++);
+ /* Enter into cache */
lookup_fhandle (p, len, npp);
return p + len / sizeof (int);
}
Index: nfs/nfs.h
===================================================================
RCS file: /cvs/hurd/nfs/nfs.h,v
retrieving revision 1.16
diff -u -r1.16 nfs.h
--- nfs/nfs.h 1999/07/11 01:40:28 1.16
+++ nfs/nfs.h 2000/12/27 04:12:31
@@ -15,6 +15,9 @@
along with this program; if not, write to the Free Software
Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. */
+#ifndef NFS_NFS_H
+#define NFS_NFS_H
+
#include <sys/stat.h>
#include <sys/types.h>
#include <sys/mman.h>
@@ -26,11 +29,11 @@
{
size_t size;
- /* Leave enough room for the biggest possible fhandle. */
+ /* Leave enough room for the largest possible fhandle. */
char data[NFS3_FHSIZE];
};
-/* One of these exists for private data needed by the client for each
+/* There exists one of there for the private data needed by each client
node. */
struct netnode
{
@@ -66,8 +69,8 @@
struct user_pager_info *fileinfo;
/* If this node has been renamed by "deletion" then
- this is the directory and name in that directory which
- is holding the node */
+ this is the directory and the name in that directory
+ which is holding the node */
struct node *dead_dir;
char *dead_name;
};
@@ -150,7 +153,7 @@
extern int protocol_version;
-/* Count how many four-byte chunks it takss to hold LEN bytes. */
+/* Count how many four-byte chunks it takes to hold LEN bytes. */
#define INTSIZE(len) (((len)+3)>>2)
@@ -193,3 +196,5 @@
void purge_lookup_cache (struct node *, char *, size_t);
struct node *check_lookup_cache (struct node *, char *);
void purge_lookup_cache_node (struct node *);
+
+#endif /* NFS_NFS_H */
Index: nfs/ops.c
===================================================================
RCS file: /cvs/hurd/nfs/ops.c,v
retrieving revision 1.40
diff -u -r1.40 ops.c
--- nfs/ops.c 2000/11/26 22:37:46 1.40
+++ nfs/ops.c 2000/12/27 04:12:35
@@ -25,9 +25,9 @@
#include <unistd.h>
#include <maptime.h>
-/* We have fresh stat information for NP; the fattr structure is at
- P. Update our entry. Return the address of the next int after
- the fattr structure. */
+/* We have fresh stat information for NP; the file attribute (fattr)
+ structure is at P. Update our entry. Return the address of the next
+ int after the fattr structure. */
int *
register_fresh_stat (struct node *np, int *p)
{
@@ -83,12 +83,12 @@
int *
process_returned_stat (struct node *np, int *p, int mod)
{
- int attrs_exist;
-
if (protocol_version == 2)
return register_fresh_stat (np, p);
else
{
+ int attrs_exist;
+
attrs_exist = ntohl (*p++);
if (attrs_exist)
p = register_fresh_stat (np, p);
@@ -144,6 +144,9 @@
p = nfs_initialize_rpc (NFSPROC_GETATTR (protocol_version),
(struct iouser *) -1, 0, &rpcbuf, np, -1);
+ if (! p)
+ return errno;
+
p = xdr_encode_fhandle (p, &np->nn->handle);
err = conduct_rpc (&rpcbuf, &p);
@@ -168,6 +171,9 @@
p = nfs_initialize_rpc (NFSPROC_SETATTR (protocol_version),
cred, 0, &rpcbuf, np, gid);
+ if (! p)
+ return errno;
+
p = xdr_encode_fhandle (p, &np->nn->handle);
p = xdr_encode_sattr_ids (p, uid, gid);
if (protocol_version == 3)
@@ -210,6 +216,8 @@
err = netfs_validate_stat (np, cred);
if (err)
return err;
+
+ /* Has the file type changed? (e.g. from symlink to directory) */
if ((mode & S_IFMT) != (np->nn_stat.st_mode & S_IFMT))
{
char *f = 0;
@@ -242,6 +250,9 @@
p = nfs_initialize_rpc (NFSPROC_SETATTR (protocol_version),
cred, 0, &rpcbuf, np, -1);
+ if (! p)
+ return errno;
+
p = xdr_encode_fhandle (p, &np->nn->handle);
p = xdr_encode_sattr_mode (p, mode);
if (protocol_version == 3)
@@ -291,6 +302,9 @@
p = nfs_initialize_rpc (NFSPROC_SETATTR (protocol_version),
cred, 0, &rpcbuf, np, -1);
+ if (! p)
+ return errno;
+
p = xdr_encode_fhandle (p, &np->nn->handle);
p = xdr_encode_sattr_times (p,
atime ?: ¤t,
@@ -322,6 +336,9 @@
p = nfs_initialize_rpc (NFSPROC_SETATTR (protocol_version),
cred, 0, &rpcbuf, np, -1);
+ if (! p)
+ return errno;
+
p = xdr_encode_fhandle (p, &np->nn->handle);
p = xdr_encode_sattr_size (p, size);
if (protocol_version == 3)
@@ -344,12 +361,9 @@
does have the file open for writing. */
if (err == EACCES)
{
- err = netfs_validate_stat (np, cred);
- if (!err && np->nn_stat.st_size == size)
+ int error = netfs_validate_stat (np, cred);
+ if (!error && np->nn_stat.st_size == size)
err = 0;
- else
- /* Never mind, put the old error back */
- err = EACCES;
}
free (rpcbuf);
@@ -367,6 +381,9 @@
error_t err;
p = nfs_initialize_rpc (NFS2PROC_STATFS, cred, 0, &rpcbuf, np, -1);
+ if (! p)
+ return errno;
+
p = xdr_encode_fhandle (p, &np->nn->handle);
err = conduct_rpc (&rpcbuf, &p);
@@ -429,6 +446,9 @@
p = nfs_initialize_rpc (NFSPROC_READ (protocol_version),
cred, 0, &rpcbuf, np, -1);
+ if (! p)
+ return errno;
+
p = xdr_encode_fhandle (p, &np->nn->handle);
*p++ = htonl (offset);
*p++ = htonl (thisamt);
@@ -458,7 +478,7 @@
else
eof = (trans_len < thisamt);
- bcopy (p, data, trans_len);
+ memcpy (data, p, trans_len);
free (rpcbuf);
data += trans_len;
@@ -495,6 +515,9 @@
p = nfs_initialize_rpc (NFSPROC_WRITE (protocol_version),
cred, thisamt, &rpcbuf, np, -1);
+ if (! p)
+ return errno;
+
p = xdr_encode_fhandle (p, &np->nn->handle);
if (protocol_version == 2)
*p++ = 0;
@@ -524,24 +547,23 @@
/* assume it wrote the whole thing */
count = thisamt;
- free (rpcbuf);
amt -= count;
data += count;
offset += count;
}
}
+ free (rpcbuf);
+
if (err == EINTR && amt != *len)
{
*len -= amt;
- free (rpcbuf);
return 0;
}
if (err)
{
*len = 0;
- free (rpcbuf);
return err;
}
}
@@ -564,6 +586,9 @@
p = nfs_initialize_rpc (NFSPROC_LOOKUP (protocol_version),
cred, 0, &rpcbuf, dir, -1);
+ if (! p)
+ return errno;
+
p = xdr_encode_fhandle (p, &dir->nn->handle);
p = xdr_encode_string (p, name);
@@ -571,6 +596,8 @@
if (!err)
err = nfs_error_trans (ntohl (*p++));
+ free (rpcbuf);
+
if (!err)
return EEXIST;
else
@@ -604,13 +631,16 @@
p = nfs_initialize_rpc (NFSPROC_LOOKUP (protocol_version),
cred, 0, &rpcbuf, np, -1);
+ if (! p)
+ return errno;
+
p = xdr_encode_fhandle (p, &np->nn->handle);
p = xdr_encode_string (p, name);
/* Remember the directory handle for later cache use. */
dirlen = np->nn->handle.size;
- bcopy (np->nn->handle.data, dirhandle, dirlen);
+ memcpy (dirhandle, np->nn->handle.data, dirlen);
mutex_unlock (&np->lock);
@@ -630,7 +660,7 @@
if (*newnp)
mutex_unlock (&(*newnp)->lock);
mutex_lock (&np->lock);
- p = process_returned_stat (np, p, 0);
+ p = process_returned_stat (np, p, 0); /* XXX Do we have to lock np? */
mutex_unlock (&np->lock);
if (*newnp)
mutex_lock (&(*newnp)->lock);
@@ -672,6 +702,9 @@
p = nfs_initialize_rpc (NFSPROC_MKDIR (protocol_version),
cred, 0, &rpcbuf, np, -1);
+ if (! p)
+ return errno;
+
p = xdr_encode_fhandle (p, &np->nn->handle);
p = xdr_encode_string (p, name);
p = xdr_encode_create_state (p, mode, owner);
@@ -680,17 +713,20 @@
if (!err)
err = nfs_error_trans (ntohl (*p++));
- p = xdr_decode_fhandle (p, &newnp);
- p = process_returned_stat (newnp, p, 1);
+ if (!err)
+ {
+ p = xdr_decode_fhandle (p, &newnp);
+ p = process_returned_stat (newnp, p, 1);
- /* Did we set the owner correctly? If not, try, but ignore failures. */
- if (!netfs_validate_stat (newnp, (struct iouser *) -1)
- && newnp->nn_stat.st_uid != owner)
- netfs_attempt_chown ((struct iouser *) -1, newnp, owner,
- newnp->nn_stat.st_gid);
+ /* Did we set the owner correctly? If not, try, but ignore failures. */
+ if (!netfs_validate_stat (newnp, (struct iouser *) -1)
+ && newnp->nn_stat.st_uid != owner)
+ netfs_attempt_chown ((struct iouser *) -1, newnp, owner,
+ newnp->nn_stat.st_gid);
- /* We don't actually return this. */
- netfs_nput (newnp);
+ /* We don't actually return this. */
+ netfs_nput (newnp);
+ }
free (rpcbuf);
return err;
@@ -712,6 +748,9 @@
p = nfs_initialize_rpc (NFSPROC_RMDIR (protocol_version),
cred, 0, &rpcbuf, np, -1);
+ if (! p)
+ return errno;
+
p = xdr_encode_fhandle (p, &np->nn->handle);
p = xdr_encode_string (p, name);
@@ -758,6 +797,12 @@
mutex_lock (&dir->lock);
p = nfs_initialize_rpc (NFSPROC_LINK (protocol_version),
cred, 0, &rpcbuf, dir, -1);
+ if (! p)
+ {
+ mutex_unlock (&dir->lock);
+ return errno;
+ }
+
mutex_unlock (&dir->lock);
mutex_lock (&np->lock);
@@ -783,6 +828,12 @@
mutex_lock (&dir->lock);
p = nfs_initialize_rpc (NFSPROC_SYMLINK (protocol_version),
cred, 0, &rpcbuf, dir, -1);
+ if (! p)
+ {
+ mutex_unlock (&dir->lock);
+ return errno;
+ }
+
p = xdr_encode_fhandle (p, &dir->nn->handle);
mutex_unlock (&dir->lock);
@@ -825,6 +876,11 @@
fhandle, so we have to fetch one now. */
p = nfs_initialize_rpc (NFSPROC_LOOKUP (protocol_version),
cred, 0, &rpcbuf, dir, -1);
+ if (! p)
+ {
+ mutex_unlock (&dir->lock);
+ return errno;
+ }
p = xdr_encode_fhandle (p, &dir->nn->handle);
p = xdr_encode_string (p, name);
@@ -880,6 +936,12 @@
p = nfs_initialize_rpc (NFSPROC_CREATE (protocol_version),
cred, 0, &rpcbuf, dir, -1);
+ if (! p)
+ {
+ mutex_unlock (&dir->lock);
+ return errno;
+ }
+
p = xdr_encode_fhandle (p, &dir->nn->handle);
p = xdr_encode_string (p, name);
mutex_unlock (&dir->lock);
@@ -898,21 +960,31 @@
mutex_lock (&dir->lock);
purge_lookup_cache (dir, name, strlen (name));
+ mutex_unlock (&dir->lock); /* XXX Should this really be after the
+ _lengthy_ (blocking) conduct_rpc? */
err = conduct_rpc (&rpcbuf, &p);
if (!err)
err = nfs_error_trans (ntohl (*p++));
- mutex_unlock (&dir->lock);
- mutex_lock (&np->lock);
- p = recache_handle (p, np);
- register_fresh_stat (np, p);
- mutex_unlock (&np->lock);
+ if (!err)
+ {
+ mutex_lock (&np->lock);
+ p = recache_handle (p, np);
+ register_fresh_stat (np, p);
+ mutex_unlock (&np->lock);
+ }
+
free (rpcbuf);
}
- else
+ else /* protocol_version != 2 */
{
mutex_lock (&dir->lock);
p = nfs_initialize_rpc (NFS3PROC_MKNOD, cred, 0, &rpcbuf, dir, -1);
+ if (! p)
+ {
+ mutex_unlock (&dir->lock);
+ return errno;
+ }
p = xdr_encode_fhandle (p, &dir->nn->handle);
p = xdr_encode_string (p, name);
mutex_unlock (&dir->lock);
@@ -996,13 +1068,17 @@
/* This is the best we can do. */
name = malloc (50);
+ if (! name)
+ return ENOMEM;
do
{
sprintf (name, ".nfstmpgnu.%d", n++);
err = netfs_attempt_create_file (cred, dir, name, mode, newnp);
if (err == EEXIST)
- mutex_lock (&dir->lock);
+ mutex_lock (&dir->lock); /* XXX is this right? does create need this
+ and drop this on error? Doesn't look
+ like it. */
}
while (err == EEXIST);
@@ -1059,6 +1135,9 @@
p = nfs_initialize_rpc (NFSPROC_CREATE (protocol_version),
cred, 0, &rpcbuf, np, -1);
+ if (! p)
+ return errno;
+
p = xdr_encode_fhandle (p, &np->nn->handle);
p = xdr_encode_string (p, name);
if (protocol_version == 3)
@@ -1149,6 +1228,13 @@
mutex_unlock (&dir->lock);
newname = malloc (50);
+ if (! newname)
+ {
+ mutex_lock (&dir->lock);
+ netfs_nrele (np); /* XXX Is this the correct thing to do? */
+ return ENOMEM;
+ }
+
do
{
sprintf (newname, ".nfs%xgnu.%d", (int) np, n++);
@@ -1186,6 +1272,9 @@
p = nfs_initialize_rpc (NFSPROC_REMOVE (protocol_version),
cred, 0, &rpcbuf, dir, -1);
+ if (! p)
+ return errno;
+
p = xdr_encode_fhandle (p, &dir->nn->handle);
p = xdr_encode_string (p, name);
@@ -1250,6 +1339,12 @@
purge_lookup_cache (fromdir, fromname, strlen (fromname));
p = nfs_initialize_rpc (NFSPROC_RENAME (protocol_version),
cred, 0, &rpcbuf, fromdir, -1);
+ if (! p)
+ {
+ mutex_unlock (&fromdir->lock);
+ return errno;
+ }
+
p = xdr_encode_fhandle (p, &fromdir->nn->handle);
p = xdr_encode_string (p, fromname);
mutex_unlock (&fromdir->lock);
@@ -1264,7 +1359,7 @@
if (!err)
{
err = nfs_error_trans (ntohl (*p++));
- if (protocol_version == 3)
+ if (protocol_version == 3) /* XXX Should we add `&& !err' ? */
{
mutex_lock (&fromdir->lock);
p = process_wcc_stat (fromdir, p, !err);
@@ -1294,6 +1389,9 @@
p = nfs_initialize_rpc (NFSPROC_READLINK (protocol_version),
cred, 0, &rpcbuf, np, -1);
+ if (! p)
+ return errno;
+
p = xdr_encode_fhandle (p, &np->nn->handle);
err = conduct_rpc (&rpcbuf, &p);
@@ -1343,7 +1441,7 @@
if (protocol_version == 2)
{
- /* Hope the server means the same thing be the bits as we do. */
+ /* Hope the server means the same thing for the bits as we do. */
*types = 0;
if (fshelp_access (&np->nn_stat, S_IREAD, cred) == 0)
*types |= O_READ;
@@ -1373,6 +1471,9 @@
}
p = nfs_initialize_rpc (NFS3PROC_ACCESS, cred, 0, &rpcbuf, np, -1);
+ if (! p)
+ return errno;
+
p = xdr_encode_fhandle (p, &np->nn->handle);
*p++ = htonl (ACCESS3_READ | write_check | execute_check);
@@ -1380,7 +1481,9 @@
if (!err)
{
err = nfs_error_trans (ntohl (*p++));
- p = process_returned_stat (np, p, 0);
+ p = process_returned_stat (np, p, 0); /* XXX Should this be
+ protected by the
+ if (!err) ? */
if (!err)
{
ret = ntohl (*p++);
@@ -1554,7 +1657,11 @@
int isnext;
bufmalloced = read_size;
+
buf = malloc (bufmalloced);
+ if (! buf)
+ return ENOMEM;
+
bp = buf;
cookie = 0;
eof = 0;
@@ -1565,6 +1672,12 @@
/* Fetch new directory entries */
p = nfs_initialize_rpc (NFSPROC_READDIR (protocol_version),
cred, 0, &rpcbuf, dir, -1);
+ if (! p)
+ {
+ free (buf);
+ return errno;
+ }
+
p = xdr_encode_fhandle (p, &dir->nn->handle);
*p++ = cookie;
*p++ = ntohl (read_size);
@@ -1600,6 +1713,7 @@
char *newbuf;
newbuf = realloc (buf, bufmalloced *= 2);
+ assert (newbuf);
if (newbuf != buf)
bp = newbuf + (bp - buf);
buf = newbuf;
@@ -1681,7 +1795,7 @@
&& thisentry < totalentries;)
{
struct dirent *entry = (struct dirent *) bp;
- bcopy (bp, userdp, entry->d_reclen);
+ memcpy (userdp, bp, entry->d_reclen);
bp += entry->d_reclen;
userdp += entry->d_reclen;
entries_copied++;
Index: nfs/rpc.c
===================================================================
RCS file: /cvs/hurd/nfs/rpc.c,v
retrieving revision 1.9
diff -u -r1.9 rpc.c
--- nfs/rpc.c 1997/07/29 20:57:33 1.9
+++ nfs/rpc.c 2000/12/27 04:12:37
@@ -28,7 +28,7 @@
#include <rpc/rpc_msg.h>
#include <rpc/auth_unix.h>
-#undef malloc /* Get rid protection. */
+#undef malloc /* Get rid of the sun block */
#include <netinet/in.h>
#include <assert.h>
@@ -43,7 +43,7 @@
void *reply;
};
-/* A list of all the pending RPCs. */
+/* A list of all pending RPCs. */
static struct rpc_list *outstanding_rpcs;
/* Wake up this condition when an outstanding RPC has received a reply
@@ -56,7 +56,7 @@
/* Generate and return a new transaction ID. */
-static int
+static inline int
generate_xid ()
{
static int nextxid;
@@ -67,22 +67,31 @@
return nextxid++;
}
-/* Set up an RPC for procdeure RPC_PROC, for talking to the server
+/* Set up an RPC for procdeure RPC_PROC for talking to the server
PROGRAM of version VERSION. Allocate storage with malloc and point
*BUF at it; caller must free this when done. Allocate at least LEN
- bytes more than the usual amount for an RPC. Initialize the RPC
- credential structure with UID, GID, and SECOND_GID. (Any of those
- may be -1 to indicate that it does not apply; exactly or two of UID
- and GID must be -1, however.) */
+ bytes more than the usual amount for the RPC. Initialize the RPC
+ credential structure with UID, GID, and SECOND_GID; any of these
+ may be -1 to indicate that it does not apply, however, exactly zero
+ or two of UID and GID must be -1. The returned address is a pointer
+ to the start of the payload. If NULL is returned, an error occured
+ and the code is set in errno. */
int *
initialize_rpc (int program, int version, int rpc_proc,
size_t len, void **bufp,
uid_t uid, gid_t gid, gid_t second_gid)
{
- void *buf = malloc (len + 1024);
+ void *buf;
int *p, *lenaddr;
struct rpc_list *hdr;
+ buf = malloc (len + 1024);
+ if (! buf)
+ {
+ errno = ENOMEM;
+ return NULL;
+ }
+
/* First the struct rpc_list bit. */
hdr = buf;
hdr->reply = 0;
@@ -99,40 +108,44 @@
assert ((uid == -1) == (gid == -1));
- if (uid != -1)
+ if (uid == -1)
+ {
+ /* No authentication */
+ *p++ = htonl (AUTH_NONE);
+ *p++ = 0;
+ }
+ else
{
+ /* Unixy authentication */
*p++ = htonl (AUTH_UNIX);
+ /* The length of the message. We do not yet know what this
+ is, so, just remember where we should put it when we know */
lenaddr = p++;
*p++ = htonl (mapped_time->seconds);
p = xdr_encode_string (p, hostname);
*p++ = htonl (uid);
*p++ = htonl (gid);
- if (second_gid != -1)
+ if (second_gid == -1)
+ *p++ = 0;
+ else
{
*p++ = htonl (1);
*p++ = htonl (second_gid);
}
- else
- *p++ = 0;
*lenaddr = htonl ((p - (lenaddr + 1)) * sizeof (int));
}
- else
- {
- *p++ = htonl (AUTH_NULL);
- *p++ = 0;
- }
/* VERF field */
- *p++ = htonl (AUTH_NULL);
+ *p++ = htonl (AUTH_NONE);
*p++ = 0;
*bufp = buf;
return p;
}
-/* Remove HDR from the list of pending RPC's. rpc_list_lock must be
- held */
-static void
+/* Remove HDR from the list of pending RPC's. The rpc_list's lock
+ (OUTSTANDING_LOCK) must be held */
+static inline void
unlink_rpc (struct rpc_list *hdr)
{
*hdr->prevp = hdr->next;
@@ -140,12 +153,24 @@
hdr->next->prevp = hdr->prevp;
}
+/* Insert HDR at the head of the LIST. The rpc_list's lock
+ (OUTSTANDING_LOCK) must be held */
+static inline void
+link_rpc (struct rpc_list **list, struct rpc_list *hdr)
+{
+ hdr->next = *list;
+ if (hdr->next)
+ hdr->next->prevp = &hdr->next;
+ hdr->prevp = list;
+ *list = hdr;
+}
+
/* Send the specified RPC message. *RPCBUF is the initialized buffer
- from a previous initialize_rpc call; *PP points past the filled
- in args. Set *PP to the address of the reply contents themselves.
- The user will be expected to free *RPCBUF (which will have changed)
- when done with the reply contents. The old value of *RPCBUF will
- be freed by this routine. */
+ from a previous initialize_rpc call; *PP, the payload, points past
+ the filledin args. Set *PP to the address of the reply contents
+ themselves. The user will be expected to free *RPCBUF (which will
+ have changed) when done with the reply contents. The old value of
+ *RPCBUF will be freed by this routine. */
error_t
conduct_rpc (void **rpcbuf, int **pp)
{
@@ -162,12 +187,7 @@
mutex_lock (&outstanding_lock);
- /* Link it in */
- hdr->next = outstanding_rpcs;
- if (hdr->next)
- hdr->next->prevp = &hdr->next;
- hdr->prevp = &outstanding_rpcs;
- outstanding_rpcs = hdr;
+ link_rpc (&outstanding_rpcs, hdr);
xid = * (int *) (*rpcbuf + sizeof (struct rpc_list));
@@ -209,9 +229,12 @@
return EINTR;
}
+ /* hdr->reply will have been filled in by rpc_receive_thread,
+ if it has been filled in, then the rpc has been fulfilled,
+ otherwise, retransmit and continue to wait */
if (!hdr->reply)
{
- timeout *=2;
+ timeout *= 2;
if (timeout > max_transmit_timeout)
timeout = max_transmit_timeout;
}
@@ -224,10 +247,12 @@
*rpcbuf = hdr->reply;
free (hdr);
- /* Process the reply, dissecting errors. When we're done, set *PP to
- the rpc return contents, if there is no error. */
+ /* Process the reply, dissecting errors. When we're done and if
+ there is no error, set *PP to the rpc return contents */
p = (int *) *rpcbuf;
+ /* If the transmition id does not match that in the message,
+ something strange happened in rpc_receive_thread */
assert (*p == xid);
p++;
@@ -315,7 +340,8 @@
return err;
}
-/* Dedicated thread to wakeup rpc_wakeup once a second. */
+/* Dedicated thread to signal those waiting on rpc_wakeup
+ once a second. */
void
timeout_service_thread ()
{
@@ -333,48 +359,52 @@
void
rpc_receive_thread ()
{
- int cc;
void *buf;
- struct rpc_list *r;
- int xid;
+ /* Allocate a receive buffer */
+ buf = malloc (1024 + read_size);
+ assert (buf);
+
while (1)
{
- buf = malloc (1024 + read_size);
-
- do
- {
- cc = read (main_udp_socket, buf, 1024 + read_size);
- if (cc == -1)
- {
- perror ("nfs read");
- r = 0;
+ int cc = read (main_udp_socket, buf, 1024 + read_size);
+ if (cc == -1)
+ {
+ perror ("nfs read");
+ continue;
+ }
+ else
+ {
+ struct rpc_list *r;
+ int xid = *(int *)buf;
+
+ mutex_lock (&outstanding_lock);
+
+ /* Find the rpc that we just fulfilled */
+ for (r = outstanding_rpcs; r; r = r->next)
+ {
+ if (* (int *) &r[1] == xid)
+ {
+ unlink_rpc (r);
+ r->reply = buf;
+ condition_broadcast (&rpc_wakeup);
+ break;
+ }
}
- else
- {
- xid = *(int *)buf;
- mutex_lock (&outstanding_lock);
- for (r = outstanding_rpcs; r; r = r->next)
- {
- if (* (int *) &r[1] == xid)
- {
- /* Remove it from the list */
- *r->prevp = r->next;
- if (r->next)
- r->next->prevp = r->prevp;
-
- r->reply = buf;
- condition_broadcast (&rpc_wakeup);
- break;
- }
- }
-#if notanymore
- if (!r)
- fprintf (stderr, "NFS dropping reply xid %d\n", xid);
+#if 0
+ if (! r)
+ fprintf (stderr, "NFS dropping reply xid %d\n", xid);
#endif
- mutex_unlock (&outstanding_lock);
+ mutex_unlock (&outstanding_lock);
+
+ /* If r is not null then we had a message from a pending
+ (i.e. known) rpc. Thus, it was fulfilled and if we
+ want to get another request, a new buffer is needed */
+ if (r)
+ {
+ buf = malloc (1024 + read_size);
+ assert (buf);
}
- }
- while (!r);
+ }
}
}
pgp0xxrfPb6up.pgp
Description: PGP signature
| [Prev in Thread] |
Current Thread |
[Next in Thread] |
- [PATCH] nfs audit,
Neal H Walfield <=