The branch main has been updated by glebius:
URL:
https://cgit.FreeBSD.org/src/commit/?id=7f39f03c4d9a138f84a08931b2a6c016521cacf5
commit 7f39f03c4d9a138f84a08931b2a6c016521cacf5
Author: Gleb Smirnoff <gleb...@freebsd.org>
AuthorDate: 2025-01-06 16:22:14 +0000
Commit: Gleb Smirnoff <gleb...@freebsd.org>
CommitDate: 2025-01-06 16:22:14 +0000
libc/xdr: remove bogus lseek(2) for xdr streams
Doing some debugging I noticed that applications using rpc(3) would often
make lseek(2) on a totally bogus file descriptor, that looks more like a
pointer. So, what happens here is that xdrrec type xdr doesn't keep a
track of how many bytes were sent/received on the stream and tries to
obtain this number via lseek(2). Then it adds/subtracts the offset in the
internal buffer from the obtained number. This code originates from the
original Sun RPC import in 1994. However, it was not a working code even
if Solaris would support lseek(2) on a socket, because it was passing not
the file descriptor, but a pointer to opaque data from upper RPC layer.
It could be that previously (before import to FreeBSD) code was correct,
but the Solaris 8 documentation says that lseek(2) on socket isn't
supported [1]. Maybe supported on older Solaris?
Anyway, this lseek(2) never worked and xdr_getpos() would always fail on
xdrrec object, until 8f55a568f69c5 in 2008 it was slightly fixed to
tolerate failure of lseek(2) and return a correct value within the small
internal buffer for XDR_ENCODE mode and a an incorrect (negative to
unsigned) result for XDR_DECODE. It seems no consumer ever calls
xdr_getpos()/xdr_setpos() on this kind of descriptor when in XDR_DECODE
mode.
So, remove this lseek(2) and preserve operation within the small buffer
only. Supposedly fix the operation for XDR_DECODE mode. Note that there
is no use and no test coverage for the XDR_DECODE.
Note that xdr(3) manual page already documents limitations for
xdr_getpos() and xdr_setpos() for the stream type objects.
[1]
https://docs.oracle.com/cd/E19109-01/tsolaris8/835-8003/6ruu1b0or/index.html
Reviewed by: asomers, markj
Differential Revision: https://reviews.freebsd.org/D48205
---
lib/libc/xdr/xdr_rec.c | 69 +++++++++++++++++++++++++-------------------------
1 file changed, 35 insertions(+), 34 deletions(-)
diff --git a/lib/libc/xdr/xdr_rec.c b/lib/libc/xdr/xdr_rec.c
index f1167fdeaa65..7dc9bbb31ec3 100644
--- a/lib/libc/xdr/xdr_rec.c
+++ b/lib/libc/xdr/xdr_rec.c
@@ -318,27 +318,30 @@ xdrrec_putbytes(XDR *xdrs, const char *addr, u_int len)
return (TRUE);
}
+/*
+ * XXX: xdrrec operates on a TCP stream and doesn't keep record of how many
+ * bytes were sent/received overall. Thus, the XDR_GETPOS() and XDR_SETPOS()
+ * can operate only within small internal buffer. So far, the limited set of
+ * consumers of this xdr are fine with that. It also seems that methods are
+ * never called in the XDR_DECODE mode.
+ */
static u_int
xdrrec_getpos(XDR *xdrs)
{
RECSTREAM *rstrm = (RECSTREAM *)xdrs->x_private;
- off_t pos;
+ ptrdiff_t pos;
- pos = lseek((int)(u_long)rstrm->tcp_handle, (off_t)0, 1);
- if (pos == -1)
- pos = 0;
switch (xdrs->x_op) {
-
case XDR_ENCODE:
- pos += rstrm->out_finger - rstrm->out_base;
+ pos = rstrm->out_finger - rstrm->out_base;
break;
case XDR_DECODE:
- pos -= rstrm->in_boundry - rstrm->in_finger;
+ pos = rstrm->in_finger - rstrm->in_base;
break;
- default:
- pos = (off_t) -1;
+ case XDR_FREE:
+ pos = -1;
break;
}
return ((u_int) pos);
@@ -352,32 +355,30 @@ xdrrec_setpos(XDR *xdrs, u_int pos)
int delta = currpos - pos;
char *newpos;
- if ((int)currpos != -1)
- switch (xdrs->x_op) {
-
- case XDR_ENCODE:
- newpos = rstrm->out_finger - delta;
- if ((newpos > (char *)(void *)(rstrm->frag_header)) &&
- (newpos < rstrm->out_boundry)) {
- rstrm->out_finger = newpos;
- return (TRUE);
- }
- break;
-
- case XDR_DECODE:
- newpos = rstrm->in_finger - delta;
- if ((delta < (int)(rstrm->fbtbc)) &&
- (newpos <= rstrm->in_boundry) &&
- (newpos >= rstrm->in_base)) {
- rstrm->in_finger = newpos;
- rstrm->fbtbc -= delta;
- return (TRUE);
- }
- break;
-
- case XDR_FREE:
- break;
+ switch (xdrs->x_op) {
+ case XDR_ENCODE:
+ newpos = rstrm->out_finger - delta;
+ if ((newpos > (char *)(void *)(rstrm->frag_header)) &&
+ (newpos < rstrm->out_boundry)) {
+ rstrm->out_finger = newpos;
+ return (TRUE);
}
+ break;
+
+ case XDR_DECODE:
+ newpos = rstrm->in_finger - delta;
+ if ((delta < (int)(rstrm->fbtbc)) &&
+ (newpos <= rstrm->in_boundry) &&
+ (newpos >= rstrm->in_base)) {
+ rstrm->in_finger = newpos;
+ rstrm->fbtbc -= delta;
+ return (TRUE);
+ }
+ break;
+
+ case XDR_FREE:
+ break;
+ }
return (FALSE);
}
