> > If anyone is interested in discussing this further, let me know. I
> > have
> > two separate patches which 'fix' this, but I don't have a deep
> > understanding of the code, so I can't say for sure that either is
> > correct.
> If this is the rounding error I was under the impression it was in the
> tree as fed to Linus. Can you drop me those two diffs again to check?
The rounding error (actually value scaling error) is indeed in the tree as
of pre-2 or so. That bug, while obviously correct was actually a red
herring, although clearly in the same code path as this one. This bug is
a bit more complex. There is a simple workaround that works for me but is
definitely a band-aid. The actual fix *seems* fairly clear, but I can't
say for a fact that it would not have side effects.
Here is a cut-and-paste of an email I sent to the G. Allen Morris III, HJ
and the list a while back that includes an explanation and the 'works for
me' patch.
Without this patch, all cached replies end up being stored as 1 byte long,
which is total rubbish, and using tcpdump you can see for yourself the
damage.
I'm not prepared to post the other patch as it hasn't been even compiled
let alone tested, wheras this one works where the unpatched knfsd fails.
- cut -
The bug is this: typical usage of a 'struct svc_buf' (when adding data) is
to advance the buf member along with incrementing the len member. For
example, here is the svc_putlong macro (from
linux/include/linux/sunrpc/svc.h)
#define svc_putlong(resp, val) { *(resp)->buf++ = (val); (resp)->len++; }
The reply cache assumes this behavior i.e. assumes that the buf pointer
has been incremented along with all data added. However, the routines in
linux/fs/nfsd/nfsxdr.c do not advance this pointer. In particular, it
seems like this pointer is *not* being advanced where it could obviously
done in xdr_ressize_check.
I assumed that this pointer must be not-incremented for a reason, so I
fixed the nfscache.c where the length calculation is done. This works for
me, but I am unsure if a small fix to xdr_ressize_check would be more
appropriate.
Here's the diff against 2.2.0-pre7. The solution was to ignore the buf
member (which is wrong) and look at the len member which *is* fixed up in
xdr_ressize_check.
--- linux/fs/nfsd/nfscache.c.orig Fri Jan 8 13:04:29 1999
+++ linux/fs/nfsd/nfscache.c Tue Jan 12 08:08:56 1999
@@ -268,8 +268,9 @@
if (!(rp = rqstp->rq_cacherep) || cache_disabled)
return;
+ len = resp->len - (statp - resp->base);
/* Don't cache excessive amounts of data and XDR failures */
- if (!statp || (len = resp->buf - statp) > (256 >> 2)) {
+ if (!statp || len > (256 >> 2)) {
rp->c_state = RC_UNUSED;
return;
}
-- /==============================\ | David Mansfield | | david@cobite.com | \==============================/
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/