[PATCH 1/1] fsidd: require root credentials on abstract socket

[PATCH 1/1] fsidd: require root credentials on abstract socket

[Olga Kornievskaia]

fsidd listens on an abstract UNIX domain socket which has no
filesystem-level access control.  Any local unprivileged user can
connect and issue mutating commands (get_or_create_fsidnum) that
grow the reexport database without limit, degrading mountd
performance.

Add a SO_PEERCRED check to reject connections from non-root
clients.  The only legitimate client is mountd, which runs as root.

Fixes: 6fd2732d ("export: Add fsidd")
Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>
---
 support/reexport/fsidd.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/support/reexport/fsidd.c b/support/reexport/fsidd.c
index 9b80b451..c714386e 100644
--- a/support/reexport/fsidd.c
+++ b/support/reexport/fsidd.c
@@ -7,6 +7,7 @@
 #include <dlfcn.h>
 #endif
 #include <event2/event.h>
+#include <sys/socket.h>
 #include <sys/un.h>
 #include <unistd.h>
 
@@ -147,6 +148,8 @@ static void srv_cb(evutil_socket_t fd, short ev, void *d)
 {
 	int cl = accept4(fd, NULL, NULL, SOCK_NONBLOCK);
 	struct event *client_ev;
+	struct ucred cred;
+	socklen_t clen = sizeof(cred);
 
 	if (cl == -1) {
 		if (errno == EMFILE || errno == ENFILE || errno == ENOMEM ||
@@ -162,6 +165,12 @@ static void srv_cb(evutil_socket_t fd, short ev, void *d)
 	(void)ev;
 	(void)d;
 
+	if (getsockopt(cl, SOL_SOCKET, SO_PEERCRED, &cred, &clen) < 0 ||
+	    cred.uid != 0) {
+		close(cl);
+		return;
+	}
+
 	client_ev = event_new(evbase, cl, EV_READ | EV_PERSIST | EV_CLOSED, client_cb, event_self_cbarg());
 	if (!client_ev || event_add(client_ev, NULL) == -1) {
 		if (client_ev)
-- 
2.52.0

[PATCH 1/3] libtirpc: Fix use-after-free in xdr_pmaplist() XDR_FREE path

[Olga Kornievskaia]

The XDR_FREE loop saved a pointer to the pml_next field inside the
struct being freed (next = &((*rp)->pml_next)), then dereferenced it
on the next iteration. This reads freed memory.

Adopt the same pattern xdr_rpcblist_ptr() already uses: save the
value of the next pointer before freeing, then point rp at a stack
copy.

Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>
---
 src/pmap_prot2.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/src/pmap_prot2.c b/src/pmap_prot2.c
index bbac74f..7611bd5 100644
--- a/src/pmap_prot2.c
+++ b/src/pmap_prot2.c
@@ -88,7 +88,8 @@ xdr_pmaplist(XDR *xdrs, struct pmaplist **rp)
 	 */
 	bool_t more_elements;
 	int freeing;
-	struct pmaplist **next	= NULL; /* pacify gcc */
+	struct pmaplist *next = NULL;
+	struct pmaplist *next_copy;
 
 	assert(xdrs != NULL);
 	assert(rp != NULL);
@@ -107,11 +108,16 @@ xdr_pmaplist(XDR *xdrs, struct pmaplist **rp)
 		 * before we free the current object ...
 		 */
 		if (freeing)
-			next = &((*rp)->pml_next); 
+			next = (*rp)->pml_next;
 		if (! xdr_reference(xdrs, (caddr_t *)rp,
 		    (u_int)sizeof(struct pmaplist), (xdrproc_t)xdr_pmap))
 			return (FALSE);
-		rp = (freeing) ? next : &((*rp)->pml_next);
+		if (freeing) {
+			next_copy = next;
+			rp = &next_copy;
+		} else {
+			rp = &((*rp)->pml_next);
+		}
 	}
 }
 
-- 
2.52.0

[PATCH 2/3] libtirpc: limit XDR decode node count

[Olga Kornievskaia]

xdr_pmaplist(), xdr_rpcblist_ptr() and xdr_rpcb_entry_list_ptr()
decode linked lists with no upper bound on element count. A malicious
server can stream arbitrarily many list nodes, forcing unbounded heap
allocation in any client that decodes the response (rpcinfo,
rpcb_getmaps() callers).

Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>
---
 src/pmap_prot2.c |  9 ++++++++-
 src/rpcb_prot.c  | 16 ++++++++++++++++
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/src/pmap_prot2.c b/src/pmap_prot2.c
index 7611bd5..d043f34 100644
--- a/src/pmap_prot2.c
+++ b/src/pmap_prot2.c
@@ -39,6 +39,8 @@
 #include <rpc/xdr.h>
 #include <rpc/pmap_prot.h>
 
+/* Cap decoded list length to prevent memory exhaustion from malicious data */
+#define PMAPLIST_DECODE_MAX_NODES 1024
 
 /*
  * What is going on with linked lists? (!)
@@ -87,7 +89,7 @@ xdr_pmaplist(XDR *xdrs, struct pmaplist **rp)
 	 * xdr_bool when the direction is XDR_DECODE.
 	 */
 	bool_t more_elements;
-	int freeing;
+	int freeing, node_count = 0;
 	struct pmaplist *next = NULL;
 	struct pmaplist *next_copy;
 
@@ -102,6 +104,11 @@ xdr_pmaplist(XDR *xdrs, struct pmaplist **rp)
 			return (FALSE);
 		if (! more_elements)
 			return (TRUE);  /* we are done */
+		if (xdrs->x_op == XDR_DECODE &&
+			++node_count > PMAPLIST_DECODE_MAX_NODES) {
+			return (FALSE);
+		}
+
 		/*
 		 * the unfortunate side effect of non-recursion is that in
 		 * the case of freeing we must remember the next object
diff --git a/src/rpcb_prot.c b/src/rpcb_prot.c
index 809feb0..0b0574f 100644
--- a/src/rpcb_prot.c
+++ b/src/rpcb_prot.c
@@ -89,6 +89,9 @@ xdr_rpcb(
  * serialize the rpcb elements.
  */
 
+/* Cap decoded list length to prevent memory exhaustion from malicious data */
+#define RPCBLIST_DECODE_MAX_NODES 1024
+
 bool_t
 xdr_rpcblist_ptr(
 	XDR *xdrs,
@@ -101,6 +104,7 @@ xdr_rpcblist_ptr(
 	 */
 	bool_t more_elements;
 	int freeing = (xdrs->x_op == XDR_FREE);
+	int node_count = 0;
 	rpcblist_ptr next;
 	rpcblist_ptr next_copy;
 
@@ -113,6 +117,10 @@ xdr_rpcblist_ptr(
 		if (! more_elements) {
 			return (TRUE);  /* we are done */
 		}
+		if (xdrs->x_op == XDR_DECODE &&
+		    ++node_count > RPCBLIST_DECODE_MAX_NODES) {
+			return (FALSE);
+		}
 		/*
 		 * the unfortunate side effect of non-recursion is that in
 		 * the case of freeing we must remember the next object
@@ -178,6 +186,9 @@ xdr_rpcb_entry(
 	return (TRUE);
 }
 
+/* Cap decoded list length to prevent memory exhaustion from malicious data */
+#define RPCB_ENTRY_LIST_DECODE_MAX_NODES 1024
+
 bool_t
 xdr_rpcb_entry_list_ptr(
 	XDR *xdrs,
@@ -190,6 +201,7 @@ xdr_rpcb_entry_list_ptr(
 	 */
 	bool_t more_elements;
 	int freeing = (xdrs->x_op == XDR_FREE);
+	int node_count = 0;
 	rpcb_entry_list_ptr next;
 	rpcb_entry_list_ptr next_copy;
 
@@ -202,6 +214,10 @@ xdr_rpcb_entry_list_ptr(
 		if (! more_elements) {
 			return (TRUE);  /* we are done */
 		}
+		if (xdrs->x_op == XDR_DECODE &&
+		    ++node_count > RPCB_ENTRY_LIST_DECODE_MAX_NODES) {
+			return (FALSE);
+		}
 		/*
 		 * the unfortunate side effect of non-recursion is that in
 		 * the case of freeing we must remember the next object
-- 
2.52.0

[PATCH 3/3] libtirpc: Bound maxsize in xdr_rpc_gss_unwrap_data() decode calls

[Olga Kornievskaia]

xdr_rpc_gss_unwrap_data() passes (u_int)-1 as maxsize to
xdr_rpc_gss_buf() for all three decode calls (databody_integ,
checksum, databody_priv). This disables the length check in
xdr_bytes(), allowing a fabricated length field in a small RPC
message to trigger a multi-GB allocation before GSS verification.

Replace (u_int)-1 with RPCSEC_GSS_MAX_UNWRAP (16 MB). The encode
path already uses bounded values (wrapbuf.length + RPC_SLACK_SPACE).

Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>
---
 src/authgss_prot.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/authgss_prot.c b/src/authgss_prot.c
index c2b25db..c7de1fd 100644
--- a/src/authgss_prot.c
+++ b/src/authgss_prot.c
@@ -51,6 +51,9 @@
 /* additional space needed for encoding */
 #define RPC_SLACK_SPACE 1024
 
+/* upper bound on decoded RPCSEC_GSS wrapped payload (16 MB) */
+#define RPCSEC_GSS_MAX_UNWRAP (16U * 1024U * 1024U)
+
 bool_t
 xdr_rpc_gss_buf(XDR *xdrs, gss_buffer_t buf, u_int maxsize)
 {
@@ -227,12 +230,12 @@ xdr_rpc_gss_unwrap_data(XDR *xdrs, xdrproc_t xdr_func, caddr_t xdr_ptr,
 
 	if (svc == RPCSEC_GSS_SVC_INTEGRITY) {
 		/* Decode databody_integ. */
-		if (!xdr_rpc_gss_buf(xdrs, &databuf, (u_int)-1)) {
+		if (!xdr_rpc_gss_buf(xdrs, &databuf, RPCSEC_GSS_MAX_UNWRAP)) {
 			LIBTIRPC_DEBUG(1, ("xdr_rpc_gss_unwrap_data: decode databody_integ failed"));
 			return (FALSE);
 		}
 		/* Decode checksum. */
-		if (!xdr_rpc_gss_buf(xdrs, &wrapbuf, (u_int)-1)) {
+		if (!xdr_rpc_gss_buf(xdrs, &wrapbuf, RPCSEC_GSS_MAX_UNWRAP)) {
 			gss_release_buffer(&min_stat, &databuf);
 			LIBTIRPC_DEBUG(1, ("xdr_rpc_gss_unwrap_data: decode checksum failed"));
 			return (FALSE);
-- 
2.52.0

Re: [PATCH 1/1] fsidd: require root credentials on abstract socket

[Steve Dickson]

On 9/6/26 9:13 PM, Olga Kornievskaia wrote:
> fsidd listens on an abstract UNIX domain socket which has no
> filesystem-level access control.  Any local unprivileged user can
> connect and issue mutating commands (get_or_create_fsidnum) that
> grow the reexport database without limit, degrading mountd
> performance.
> 
> Add a SO_PEERCRED check to reject connections from non-root
> clients.  The only legitimate client is mountd, which runs as root.
> 
> Fixes: 6fd2732d ("export: Add fsidd")
> Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>
> ---
>   support/reexport/fsidd.c | 9 +++++++++
>   1 file changed, 9 insertions(+)
> 
> diff --git a/support/reexport/fsidd.c b/support/reexport/fsidd.c
> index 9b80b451..c714386e 100644
> --- a/support/reexport/fsidd.c
> +++ b/support/reexport/fsidd.c
> @@ -7,6 +7,7 @@
>   #include <dlfcn.h>
>   #endif
>   #include <event2/event.h>
> +#include <sys/socket.h>
>   #include <sys/un.h>
>   #include <unistd.h>
>   
> @@ -147,6 +148,8 @@ static void srv_cb(evutil_socket_t fd, short ev, void *d)
>   {
>   	int cl = accept4(fd, NULL, NULL, SOCK_NONBLOCK);
>   	struct event *client_ev;
> +	struct ucred cred;
> +	socklen_t clen = sizeof(cred);
>   
>   	if (cl == -1) {
>   		if (errno == EMFILE || errno == ENFILE || errno == ENOMEM ||
> @@ -162,6 +165,12 @@ static void srv_cb(evutil_socket_t fd, short ev, void *d)
>   	(void)ev;
>   	(void)d;
>   
> +	if (getsockopt(cl, SOL_SOCKET, SO_PEERCRED, &cred, &clen) < 0 ||
> +	    cred.uid != 0) {
> +		close(cl);
> +		return;
> +	}
> +
>   	client_ev = event_new(evbase, cl, EV_READ | EV_PERSIST | EV_CLOSED, client_cb, event_self_cbarg());
>   	if (!client_ev || event_add(client_ev, NULL) == -1) {
>   		if (client_ev)
This didn't apply cleanly... so I'll deal with it with some other
upcoming fsidd patches

steved.

Re: [PATCH 1/3] libtirpc: Fix use-after-free in xdr_pmaplist() XDR_FREE path

[Steve Dickson]

On 9/6/26 9:13 PM, Olga Kornievskaia wrote:
> The XDR_FREE loop saved a pointer to the pml_next field inside the
> struct being freed (next = &((*rp)->pml_next)), then dereferenced it
> on the next iteration. This reads freed memory.
> 
> Adopt the same pattern xdr_rpcblist_ptr() already uses: save the
> value of the next pointer before freeing, then point rp at a stack
> copy.
> 
> Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>
Committed (tag: libtirpc-1-3-8-rc2)

steved.> ---
>   src/pmap_prot2.c | 12 +++++++++---
>   1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/src/pmap_prot2.c b/src/pmap_prot2.c
> index bbac74f..7611bd5 100644
> --- a/src/pmap_prot2.c
> +++ b/src/pmap_prot2.c
> @@ -88,7 +88,8 @@ xdr_pmaplist(XDR *xdrs, struct pmaplist **rp)
>   	 */
>   	bool_t more_elements;
>   	int freeing;
> -	struct pmaplist **next	= NULL; /* pacify gcc */
> +	struct pmaplist *next = NULL;
> +	struct pmaplist *next_copy;
>   
>   	assert(xdrs != NULL);
>   	assert(rp != NULL);
> @@ -107,11 +108,16 @@ xdr_pmaplist(XDR *xdrs, struct pmaplist **rp)
>   		 * before we free the current object ...
>   		 */
>   		if (freeing)
> -			next = &((*rp)->pml_next);
> +			next = (*rp)->pml_next;
>   		if (! xdr_reference(xdrs, (caddr_t *)rp,
>   		    (u_int)sizeof(struct pmaplist), (xdrproc_t)xdr_pmap))
>   			return (FALSE);
> -		rp = (freeing) ? next : &((*rp)->pml_next);
> +		if (freeing) {
> +			next_copy = next;
> +			rp = &next_copy;
> +		} else {
> +			rp = &((*rp)->pml_next);
> +		}
>   	}
>   }
>   

Re: [PATCH 2/3] libtirpc: limit XDR decode node count

[Steve Dickson]

On 9/6/26 9:13 PM, Olga Kornievskaia wrote:
> xdr_pmaplist(), xdr_rpcblist_ptr() and xdr_rpcb_entry_list_ptr()
> decode linked lists with no upper bound on element count. A malicious
> server can stream arbitrarily many list nodes, forcing unbounded heap
> allocation in any client that decodes the response (rpcinfo,
> rpcb_getmaps() callers).
> 
> Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>
Committed (tag: libtirpc-1-3-8-rc2)

steved.
> ---
>   src/pmap_prot2.c |  9 ++++++++-
>   src/rpcb_prot.c  | 16 ++++++++++++++++
>   2 files changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/src/pmap_prot2.c b/src/pmap_prot2.c
> index 7611bd5..d043f34 100644
> --- a/src/pmap_prot2.c
> +++ b/src/pmap_prot2.c
> @@ -39,6 +39,8 @@
>   #include <rpc/xdr.h>
>   #include <rpc/pmap_prot.h>
>   
> +/* Cap decoded list length to prevent memory exhaustion from malicious data */
> +#define PMAPLIST_DECODE_MAX_NODES 1024
>   
>   /*
>    * What is going on with linked lists? (!)
> @@ -87,7 +89,7 @@ xdr_pmaplist(XDR *xdrs, struct pmaplist **rp)
>   	 * xdr_bool when the direction is XDR_DECODE.
>   	 */
>   	bool_t more_elements;
> -	int freeing;
> +	int freeing, node_count = 0;
>   	struct pmaplist *next = NULL;
>   	struct pmaplist *next_copy;
>   
> @@ -102,6 +104,11 @@ xdr_pmaplist(XDR *xdrs, struct pmaplist **rp)
>   			return (FALSE);
>   		if (! more_elements)
>   			return (TRUE);  /* we are done */
> +		if (xdrs->x_op == XDR_DECODE &&
> +			++node_count > PMAPLIST_DECODE_MAX_NODES) {
> +			return (FALSE);
> +		}
> +
>   		/*
>   		 * the unfortunate side effect of non-recursion is that in
>   		 * the case of freeing we must remember the next object
> diff --git a/src/rpcb_prot.c b/src/rpcb_prot.c
> index 809feb0..0b0574f 100644
> --- a/src/rpcb_prot.c
> +++ b/src/rpcb_prot.c
> @@ -89,6 +89,9 @@ xdr_rpcb(
>    * serialize the rpcb elements.
>    */
>   
> +/* Cap decoded list length to prevent memory exhaustion from malicious data */
> +#define RPCBLIST_DECODE_MAX_NODES 1024
> +
>   bool_t
>   xdr_rpcblist_ptr(
>   	XDR *xdrs,
> @@ -101,6 +104,7 @@ xdr_rpcblist_ptr(
>   	 */
>   	bool_t more_elements;
>   	int freeing = (xdrs->x_op == XDR_FREE);
> +	int node_count = 0;
>   	rpcblist_ptr next;
>   	rpcblist_ptr next_copy;
>   
> @@ -113,6 +117,10 @@ xdr_rpcblist_ptr(
>   		if (! more_elements) {
>   			return (TRUE);  /* we are done */
>   		}
> +		if (xdrs->x_op == XDR_DECODE &&
> +		    ++node_count > RPCBLIST_DECODE_MAX_NODES) {
> +			return (FALSE);
> +		}
>   		/*
>   		 * the unfortunate side effect of non-recursion is that in
>   		 * the case of freeing we must remember the next object
> @@ -178,6 +186,9 @@ xdr_rpcb_entry(
>   	return (TRUE);
>   }
>   
> +/* Cap decoded list length to prevent memory exhaustion from malicious data */
> +#define RPCB_ENTRY_LIST_DECODE_MAX_NODES 1024
> +
>   bool_t
>   xdr_rpcb_entry_list_ptr(
>   	XDR *xdrs,
> @@ -190,6 +201,7 @@ xdr_rpcb_entry_list_ptr(
>   	 */
>   	bool_t more_elements;
>   	int freeing = (xdrs->x_op == XDR_FREE);
> +	int node_count = 0;
>   	rpcb_entry_list_ptr next;
>   	rpcb_entry_list_ptr next_copy;
>   
> @@ -202,6 +214,10 @@ xdr_rpcb_entry_list_ptr(
>   		if (! more_elements) {
>   			return (TRUE);  /* we are done */
>   		}
> +		if (xdrs->x_op == XDR_DECODE &&
> +		    ++node_count > RPCB_ENTRY_LIST_DECODE_MAX_NODES) {
> +			return (FALSE);
> +		}
>   		/*
>   		 * the unfortunate side effect of non-recursion is that in
>   		 * the case of freeing we must remember the next object

Re: [PATCH 3/3] libtirpc: Bound maxsize in xdr_rpc_gss_unwrap_data() decode calls

[Steve Dickson]

On 9/6/26 9:13 PM, Olga Kornievskaia wrote:
> xdr_rpc_gss_unwrap_data() passes (u_int)-1 as maxsize to
> xdr_rpc_gss_buf() for all three decode calls (databody_integ,
> checksum, databody_priv). This disables the length check in
> xdr_bytes(), allowing a fabricated length field in a small RPC
> message to trigger a multi-GB allocation before GSS verification.
> 
> Replace (u_int)-1 with RPCSEC_GSS_MAX_UNWRAP (16 MB). The encode
> path already uses bounded values (wrapbuf.length + RPC_SLACK_SPACE).
> 
> Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>
Committed (tag: libtirpc-1-3-8-rc2)

steved.
> ---
>   src/authgss_prot.c | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/src/authgss_prot.c b/src/authgss_prot.c
> index c2b25db..c7de1fd 100644
> --- a/src/authgss_prot.c
> +++ b/src/authgss_prot.c
> @@ -51,6 +51,9 @@
>   /* additional space needed for encoding */
>   #define RPC_SLACK_SPACE 1024
>   
> +/* upper bound on decoded RPCSEC_GSS wrapped payload (16 MB) */
> +#define RPCSEC_GSS_MAX_UNWRAP (16U * 1024U * 1024U)
> +
>   bool_t
>   xdr_rpc_gss_buf(XDR *xdrs, gss_buffer_t buf, u_int maxsize)
>   {
> @@ -227,12 +230,12 @@ xdr_rpc_gss_unwrap_data(XDR *xdrs, xdrproc_t xdr_func, caddr_t xdr_ptr,
>   
>   	if (svc == RPCSEC_GSS_SVC_INTEGRITY) {
>   		/* Decode databody_integ. */
> -		if (!xdr_rpc_gss_buf(xdrs, &databuf, (u_int)-1)) {
> +		if (!xdr_rpc_gss_buf(xdrs, &databuf, RPCSEC_GSS_MAX_UNWRAP)) {
>   			LIBTIRPC_DEBUG(1, ("xdr_rpc_gss_unwrap_data: decode databody_integ failed"));
>   			return (FALSE);
>   		}
>   		/* Decode checksum. */
> -		if (!xdr_rpc_gss_buf(xdrs, &wrapbuf, (u_int)-1)) {
> +		if (!xdr_rpc_gss_buf(xdrs, &wrapbuf, RPCSEC_GSS_MAX_UNWRAP)) {
>   			gss_release_buffer(&min_stat, &databuf);
>   			LIBTIRPC_DEBUG(1, ("xdr_rpc_gss_unwrap_data: decode checksum failed"));
>   			return (FALSE);