Skip to content

Commit 637dd14

Browse files
committed
ltop: fix buffer overflow in recovery status
A buffer used to display recovery status from one of the MDTs could be overflowed when the recovery status was copied from the cerebro message. The buffer was also too small to contain the entire recovery status, so that the status visible to the user was sometimes truncated. This patch fixes both issues.
1 parent 3e00a72 commit 637dd14

File tree

2 files changed

+29
-18
lines changed

2 files changed

+29
-18
lines changed

liblmt/common.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11

22

3-
#define RECOVERY_STR_SIZE 32
3+
#define RECOVERY_STR_SIZE 64
44

55
int
66
get_recovstr (pctx_t ctx, char *name, char *s, int len);

utils/ltop.c

Lines changed: 28 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -576,12 +576,16 @@ _update_display_top (WINDOW *win, char *fs, List ost_data, List mdt_data,
576576
mdtstat_t *m;
577577

578578
/*
579-
* Recovery status fits between filesystem name and indicators like "RECORDING"
580-
* The former takes up 12+strlen(fs) columns
581-
* The indicators are displayed starting at column 68
582-
* Some blank spaces on either side are desirable.
579+
* Recovery status fits between filesystem name and indicators
580+
* like "RECORDING". The former takes up 12+strlen(fs) columns.
581+
* The indicators are displayed starting at column 68. Some blank
582+
* spaces on either side are desirable.
583+
*
584+
* We also need to make sure we don't overflow the recovery_status.
583585
*/
584586
recov_status_len = 68 - (12 + strlen(fs) + 4);
587+
recov_status_len = (RECOVERY_STR_SIZE < recov_status_len ?
588+
RECOVERY_STR_SIZE : recov_status_len);
585589

586590
itr = list_iterator_create (ost_data);
587591
while ((o = list_next (itr))) {
@@ -607,20 +611,27 @@ _update_display_top (WINDOW *win, char *fs, List ost_data, List mdt_data,
607611
getxattr += sample_rate (m->getxattr, tnow);
608612
minodes_free += sample_val (m->inodes_free, tnow) / (1024*1024);
609613
minodes_total += sample_val (m->inodes_total, tnow) / (1024*1024);
610-
if (m->recov_status && strstr(m->recov_status,"RECOV")) {
611-
/*
612-
* Multiple MDTs may be in recovery, but display room is
613-
* limited. We print the full status of the first MDT in recovery
614-
* we find. This allows the user to tell whether the count of
615-
* reconnected clients is increasing.
616-
*/
617-
if (recovery_status[0] == '\0')
618-
snprintf(recovery_status, recov_status_len, "MDT%s %s",
619-
m->name, m->recov_status);
620-
}
621614

622-
if (m->mdt_metric_timestamp > trcv)
623-
trcv = m->mdt_metric_timestamp;
615+
/*
616+
* recovery_status is just a string, and has no timestamp.
617+
*/
618+
if ((tnow - m->common.tgt_metric_timestamp) < stale_secs) {
619+
if (m->common.recov_status && strstr(m->common.recov_status,"RECOV")) {
620+
/*
621+
* Multiple MDTs may be in recovery, but display room
622+
* is limited. We print the full status of the first
623+
* MDT in recovery we find. This allows the user to
624+
* tell whether the count of reconnected clients is
625+
* increasing.
626+
*/
627+
if (recovery_status[0] == '\0')
628+
snprintf(recovery_status, recov_status_len, "MDT%s %s",
629+
m->common.name, m->common.recov_status);
630+
}
631+
}
632+
633+
if (m->common.tgt_metric_timestamp > trcv)
634+
trcv = m->common.tgt_metric_timestamp;
624635
}
625636
list_iterator_destroy (itr);
626637

0 commit comments

Comments
 (0)