| Message ID | 20210323135314.1595521-1-cmaiolino@redhat.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [V2] xfs_logprint: Fix buffer overflow printing quotaoff | expand |
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 >
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
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(-)