[PATCH V2] xfs_logprint: Fix buffer overflow printing quotaoff

[PATCH V2] xfs_logprint: Fix buffer overflow printing quotaoff

[Carlos Maiolino]

xlog_recover_print_quotaoff() was using a static buffer to aggregate
quota option strings to be printed at the end. The buffer size was
miscalculated and when printing all 3 flags, a buffer overflow occurs
crashing xfs_logprint, like:

QOFF: cnt:1 total:1 a:0x560530ff3bb0 len:160
*** buffer overflow detected ***: terminated
Aborted (core dumped)

Fix this by removing the static buffer and using printf() directly to
print each flag. Also add a trailling space before each flag, so they
are a bit more readable on the output.

Reported-by: Eric Sandeen <sandeen@sandeen.net>
Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
Changelog:

 - V2:
	Update strings removing the "QUOTA" of each printf, resulting
	in: "USER GROUP PROJECT"

 logprint/log_print_all.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/logprint/log_print_all.c b/logprint/log_print_all.c
index 20f2a445..c9c453f6 100644
--- a/logprint/log_print_all.c
+++ b/logprint/log_print_all.c
@@ -186,18 +186,18 @@ xlog_recover_print_quotaoff(
 	struct xlog_recover_item *item)
 {
 	xfs_qoff_logformat_t	*qoff_f;
-	char			str[32] = { 0 };
 
 	qoff_f = (xfs_qoff_logformat_t *)item->ri_buf[0].i_addr;
+
 	ASSERT(qoff_f);
+	printf(_("\tQUOTAOFF: #regs:%d   type:"), qoff_f->qf_size);
 	if (qoff_f->qf_flags & XFS_UQUOTA_ACCT)
-		strcat(str, "USER QUOTA");
+		printf(" USER");
 	if (qoff_f->qf_flags & XFS_GQUOTA_ACCT)
-		strcat(str, "GROUP QUOTA");
+		printf(" GROUP");
 	if (qoff_f->qf_flags & XFS_PQUOTA_ACCT)
-		strcat(str, "PROJECT QUOTA");
-	printf(_("\tQUOTAOFF: #regs:%d   type:%s\n"),
-	       qoff_f->qf_size, str);
+		printf(" PROJECT");
+	printf("\n");
 }
 
 STATIC void
-- 
2.29.2

Re: [PATCH V2] xfs_logprint: Fix buffer overflow printing quotaoff

[Darrick J. Wong]

On Tue, Mar 23, 2021 at 02:53:14PM +0100, Carlos Maiolino wrote:
> xlog_recover_print_quotaoff() was using a static buffer to aggregate
> quota option strings to be printed at the end. The buffer size was
> miscalculated and when printing all 3 flags, a buffer overflow occurs
> crashing xfs_logprint, like:
> 
> QOFF: cnt:1 total:1 a:0x560530ff3bb0 len:160
> *** buffer overflow detected ***: terminated
> Aborted (core dumped)
> 
> Fix this by removing the static buffer and using printf() directly to
> print each flag. Also add a trailling space before each flag, so they
> are a bit more readable on the output.
> 
> Reported-by: Eric Sandeen <sandeen@sandeen.net>
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>

Seems reasonable to me,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
> Changelog:
> 
>  - V2:
> 	Update strings removing the "QUOTA" of each printf, resulting
> 	in: "USER GROUP PROJECT"
> 
>  logprint/log_print_all.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/logprint/log_print_all.c b/logprint/log_print_all.c
> index 20f2a445..c9c453f6 100644
> --- a/logprint/log_print_all.c
> +++ b/logprint/log_print_all.c
> @@ -186,18 +186,18 @@ xlog_recover_print_quotaoff(
>  	struct xlog_recover_item *item)
>  {
>  	xfs_qoff_logformat_t	*qoff_f;
> -	char			str[32] = { 0 };
>  
>  	qoff_f = (xfs_qoff_logformat_t *)item->ri_buf[0].i_addr;
> +
>  	ASSERT(qoff_f);
> +	printf(_("\tQUOTAOFF: #regs:%d   type:"), qoff_f->qf_size);
>  	if (qoff_f->qf_flags & XFS_UQUOTA_ACCT)
> -		strcat(str, "USER QUOTA");
> +		printf(" USER");
>  	if (qoff_f->qf_flags & XFS_GQUOTA_ACCT)
> -		strcat(str, "GROUP QUOTA");
> +		printf(" GROUP");
>  	if (qoff_f->qf_flags & XFS_PQUOTA_ACCT)
> -		strcat(str, "PROJECT QUOTA");
> -	printf(_("\tQUOTAOFF: #regs:%d   type:%s\n"),
> -	       qoff_f->qf_size, str);
> +		printf(" PROJECT");
> +	printf("\n");
>  }
>  
>  STATIC void
> -- 
> 2.29.2
>