On Apr 27, 2009, at 7:51 PM, J. Bruce Fields wrote:
On Mon, Apr 27, 2009 at 12:49:07PM -0400, Chuck Lever wrote:
On Apr 25, 2009, at 6:17 PM, J. Bruce Fields wrote:
On Thu, Apr 23, 2009 at 07:31:25PM -0400, Chuck Lever wrote:
The svc_addr_len() helper function returns -EAFNOSUPPORT if it
doesn't
recognize the address family of the passed-in socket address.
However,
the return type of this function is size_t, which means -
EAFNOSUPPORT
is turned into a very large positive value in this case.
The check in svc_udp_recvfrom() to see if the return value is less
than zero therefore won't work at all.
Additionally, handle_connect_req() passes this value directly to
memset(). This could cause memset() to clobber a large chunk of
memory
if svc_addr_len() has returned an error. Currently the address
family
of these addresses, however, is known to be supported long before
handle_connect_req() is called, so this isn't a real risk.
Change the error return value of svc_addr_len() to zero, which fits
in
the range of size_t, and is safer to pass to memset() directly.
Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
---
include/linux/sunrpc/svc_xprt.h | 5 +++--
net/sunrpc/svcsock.c | 7 ++++---
2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/include/linux/sunrpc/svc_xprt.h b/include/linux/
sunrpc/
svc_xprt.h
index 0d9cb6e..d790c52 100644
--- a/include/linux/sunrpc/svc_xprt.h
+++ b/include/linux/sunrpc/svc_xprt.h
@@ -118,7 +118,7 @@ static inline unsigned short
svc_addr_port(const
struct sockaddr *sa)
return 0;
}
-static inline size_t svc_addr_len(struct sockaddr *sa)
+static inline size_t svc_addr_len(const struct sockaddr *sa)
{
switch (sa->sa_family) {
case AF_INET:
@@ -126,7 +126,8 @@ static inline size_t svc_addr_len(struct
sockaddr *sa)
case AF_INET6:
return sizeof(struct sockaddr_in6);
}
- return -EAFNOSUPPORT;
+
May as well stick a WARN() here too if only as a shorthand way of
documenting that this isn't meant to happen.
Sounds reasonable, but:
1. I thought BUG() was the way to document "shouldn't happen," and
That'd be OK too.
2. What about all the other static inliney places this "shouldn't
happen" ?
Such as?
nlm_cmp_addr(), nlm_privileged_requester(), svc_addr_port(),
nfs_sockaddr_match_ipaddr(), nfs_sockaddr_cmp(), nfs_server_get_key(),
nfs_set_port(), nfs_verify_server_address(), and so on.
--b.
I guess we kind of gave up on BUG'ing in all these places.
--b.
+ return 0;
}
static inline unsigned short svc_xprt_local_port(const struct
svc_xprt *xprt)
diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
index af31988..8b08328 100644
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -426,13 +426,14 @@ static int svc_udp_recvfrom(struct svc_rqst
*rqstp)
long all[SVC_PKTINFO_SPACE / sizeof(long)];
} buffer;
struct cmsghdr *cmh = &buffer.hdr;
- int err, len;
struct msghdr msg = {
.msg_name = svc_addr(rqstp),
.msg_control = cmh,
.msg_controllen = sizeof(buffer),
.msg_flags = MSG_DONTWAIT,
};
+ size_t len;
+ int err;
if (test_and_clear_bit(XPT_CHNGBUF, &svsk->sk_xprt.xpt_flags))
/* udp sockets need large rcvbuf as all pending
@@ -464,8 +465,8 @@ static int svc_udp_recvfrom(struct svc_rqst
*rqstp)
return -EAGAIN;
}
len = svc_addr_len(svc_addr(rqstp));
- if (len < 0)
- return len;
+ if (len == 0)
+ return -EAFNOSUPPORT;
rqstp->rq_addrlen = len;
if (skb->tstamp.tv64 == 0) {
skb->tstamp = ktime_get_real();
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
