git: 7f39f03c4d9a - main - libc/xdr: remove bogus lseek(2) for xdr streams

The branch main has been updated by glebius:

URL: https://cgit.FreeBSD.org/src/commit/?id=7f39f03c4d9a138f84a08931b2a6c016521cacf5

commit 7f39f03c4d9a138f84a08931b2a6c016521cacf5
Author:     Gleb Smirnoff <glebius@FreeBSD.org>
AuthorDate: 2025-01-06 16:22:14 +0000
Commit:     Gleb Smirnoff <glebius@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);
 }