Messages in this thread Patch in this message | | | Date | Sat, 23 Jan 1999 18:44:59 -0500 (EST) | From | David Mansfield <> | Subject | Re: knfsd status?? |
| |
Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> > 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/
| |