[PATCH 1/1] nfs-utils: mount: fix snprintf return value handling in error formatting

[PATCH 1/1] nfs-utils: mount: fix snprintf return value handling in error formatting

[Olga Kornievskaia]

snprintf() returns the number of bytes that would have been written
if sufficient space were available, not the number actually written.
When the formatted string exceeds PATH_MAX (e.g. due to a long
progname), the return value exceeds the buffer size. Using that
unclamped value as an index into errbuf produces an out-of-bounds
pointer, and subsequent snprintf() calls write past the buffer.

Add errbuf_clamp() to bound the position to the valid range before
it is used as a buffer index. Apply it in rpc_mount_errors(),
sys_mount_errors(), and rpc_strerror().

Fixes: 6e54f6179cb9 ("text-based mount.nfs: Add text-based error reporting function")
Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>
---
 utils/mount/error.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/utils/mount/error.c b/utils/mount/error.c
index d6cbdea1..4dace8f5 100644
--- a/utils/mount/error.c
+++ b/utils/mount/error.c
@@ -54,6 +54,21 @@ extern char *progname;
 static char errbuf[PATH_MAX];
 static char *erreob = &errbuf[PATH_MAX];
 
+/*
+ * Clamp a buffer position to the valid range for errbuf.
+ * snprintf() returns the number of bytes that would have been written,
+ * which can exceed the buffer size on truncation.  Using that unclamped
+ * value as an index into errbuf produces an out-of-bounds pointer.
+ */
+static int errbuf_clamp(int pos)
+{
+	if (pos < 0)
+		return 0;
+	if (pos >= PATH_MAX)
+		return PATH_MAX - 1;
+	return pos;
+}
+
 /* Convert RPC errors into strings */
 static int rpc_strerror(int spos)
 {
@@ -62,6 +77,8 @@ static int rpc_strerror(int spos)
 	char *ptr, *estr = clnt_sperrno(cf_stat);
 	char *tmp;
 
+	spos = errbuf_clamp(spos);
+
 	if (estr) {
 		if ((ptr = strchr(estr, ':')))
 			estr = ++ptr;
@@ -72,9 +89,9 @@ static int rpc_strerror(int spos)
 					_("System Error: %s"),
 						strerror(cf_errno));
 		else {
-			if (cf_errno) 
+			if (cf_errno)
 				pos = snprintf(tmp, (erreob - tmp),
-					_("RPC Error:%s; errno = %s"), 
+					_("RPC Error:%s; errno = %s"),
 					estr, strerror(cf_errno));
 			else
 				pos = snprintf(tmp, (erreob - tmp),
@@ -109,6 +126,7 @@ void rpc_mount_errors(char *server, int will_retry, int bg)
 				_("%s: mount to NFS server '%s' failed: "),
 					progname, server);
 
+	pos = errbuf_clamp(pos);
 	tmp = &errbuf[pos];
 	if (rpc_createerr.cf_stat == RPC_TIMEDOUT) {
 		if (will_retry)
@@ -119,6 +137,7 @@ void rpc_mount_errors(char *server, int will_retry, int bg)
 					_("timed out, giving up"));
 	} else {
 		pos += rpc_strerror(pos);
+		pos = errbuf_clamp(pos);
 		tmp = &errbuf[pos];
 		if (bg) {
 			if (will_retry)
@@ -164,6 +183,7 @@ void sys_mount_errors(char *server, int error, int will_retry, int bg)
 				_("%s: mount to NFS server '%s' failed: "),
 					progname, server);
 
+	pos = errbuf_clamp(pos);
 	tmp = &errbuf[pos];
 	if (error == ETIMEDOUT) {
 		if (will_retry)
-- 
2.52.0

Re: [PATCH 1/1] nfs-utils: mount: fix snprintf return value handling in error formatting

[Steve Dickson]

On 8/25/26 2:53 PM, Olga Kornievskaia wrote:
> snprintf() returns the number of bytes that would have been written
> if sufficient space were available, not the number actually written.
> When the formatted string exceeds PATH_MAX (e.g. due to a long
> progname), the return value exceeds the buffer size. Using that
> unclamped value as an index into errbuf produces an out-of-bounds
> pointer, and subsequent snprintf() calls write past the buffer.
> 
> Add errbuf_clamp() to bound the position to the valid range before
> it is used as a buffer index. Apply it in rpc_mount_errors(),
> sys_mount_errors(), and rpc_strerror().
> 
> Fixes: 6e54f6179cb9 ("text-based mount.nfs: Add text-based error reporting function")
> Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>
Committed... (tag: nfs-utils-2-9-3-rc2)

steved.
> ---
>   utils/mount/error.c | 24 ++++++++++++++++++++++--
>   1 file changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/utils/mount/error.c b/utils/mount/error.c
> index d6cbdea1..4dace8f5 100644
> --- a/utils/mount/error.c
> +++ b/utils/mount/error.c
> @@ -54,6 +54,21 @@ extern char *progname;
>   static char errbuf[PATH_MAX];
>   static char *erreob = &errbuf[PATH_MAX];
>   
> +/*
> + * Clamp a buffer position to the valid range for errbuf.
> + * snprintf() returns the number of bytes that would have been written,
> + * which can exceed the buffer size on truncation.  Using that unclamped
> + * value as an index into errbuf produces an out-of-bounds pointer.
> + */
> +static int errbuf_clamp(int pos)
> +{
> +	if (pos < 0)
> +		return 0;
> +	if (pos >= PATH_MAX)
> +		return PATH_MAX - 1;
> +	return pos;
> +}
> +
>   /* Convert RPC errors into strings */
>   static int rpc_strerror(int spos)
>   {
> @@ -62,6 +77,8 @@ static int rpc_strerror(int spos)
>   	char *ptr, *estr = clnt_sperrno(cf_stat);
>   	char *tmp;
>   
> +	spos = errbuf_clamp(spos);
> +
>   	if (estr) {
>   		if ((ptr = strchr(estr, ':')))
>   			estr = ++ptr;
> @@ -72,9 +89,9 @@ static int rpc_strerror(int spos)
>   					_("System Error: %s"),
>   						strerror(cf_errno));
>   		else {
> -			if (cf_errno)
> +			if (cf_errno)
>   				pos = snprintf(tmp, (erreob - tmp),
> -					_("RPC Error:%s; errno = %s"),
> +					_("RPC Error:%s; errno = %s"),
>   					estr, strerror(cf_errno));
>   			else
>   				pos = snprintf(tmp, (erreob - tmp),
> @@ -109,6 +126,7 @@ void rpc_mount_errors(char *server, int will_retry, int bg)
>   				_("%s: mount to NFS server '%s' failed: "),
>   					progname, server);
>   
> +	pos = errbuf_clamp(pos);
>   	tmp = &errbuf[pos];
>   	if (rpc_createerr.cf_stat == RPC_TIMEDOUT) {
>   		if (will_retry)
> @@ -119,6 +137,7 @@ void rpc_mount_errors(char *server, int will_retry, int bg)
>   					_("timed out, giving up"));
>   	} else {
>   		pos += rpc_strerror(pos);
> +		pos = errbuf_clamp(pos);
>   		tmp = &errbuf[pos];
>   		if (bg) {
>   			if (will_retry)
> @@ -164,6 +183,7 @@ void sys_mount_errors(char *server, int error, int will_retry, int bg)
>   				_("%s: mount to NFS server '%s' failed: "),
>   					progname, server);
>   
> +	pos = errbuf_clamp(pos);
>   	tmp = &errbuf[pos];
>   	if (error == ETIMEDOUT) {
>   		if (will_retry)