From b631a91e7e70ff609268def24e51b5bb089a5545 Mon Sep 17 00:00:00 2001 From: Frank Zago Date: Thu, 19 May 2016 17:18:18 -0400 Subject: [PATCH] LU-8171 hsm: stack overrun in hai_dump_data_field With the stock 3.10 kernel, hai_dump_data_field will do a stack buffer overrun when cat'ing /proc/fs/lustre/.../hsm/actions if an action has some data in it. hai_dump_data_field uses snprintf. But there is no check for truncation, and the value returned by snprintf is used as-is. The coordinator code calls hai_dump_data_field with 12 bytes in the buffer. The 6th byte of data is printed incompletely to make room for the terminating NUL. However snprintf still returns 2, so when hai_dump_data_field writes the final NUL, it does it outside the reserved buffer, in the 13th byte of the buffer. This stack buffer overrun hangs my VM. This doesn't happen on CentOS 6. Fix by checking that there is enough room for the next 2 characters plus the NUL terminator. Don't print half bytes. Change the format to 02X instead of .2X, which makes more sense. Signed-off-by: frank zago Change-Id: Icb6e658fc63f441b7a698f595ac71a1236ff3588 Reviewed-on: http://review.whamcloud.com/20338 Tested-by: Jenkins Reviewed-by: John L. Hammond Tested-by: Maloo Reviewed-by: Jean-Baptiste Riaux Reviewed-by: Oleg Drokin --- lustre/include/lustre/lustre_user.h | 44 ++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/lustre/include/lustre/lustre_user.h b/lustre/include/lustre/lustre_user.h index d303380..78a4cba 100644 --- a/lustre/include/lustre/lustre_user.h +++ b/lustre/include/lustre/lustre_user.h @@ -1246,34 +1246,34 @@ struct hsm_action_item { char hai_data[0]; /* variable length */ } __attribute__((packed)); -/* +/** * helper function which print in hexa the first bytes of * hai opaque field - * \param hai [IN] record to print - * \param buffer [OUT] output buffer - * \param len [IN] max buffer len + * + * \param hai [IN] record to print + * \param buffer [IN,OUT] buffer to write the hex string to + * \param len [IN] max buffer length + * * \retval buffer */ static inline char *hai_dump_data_field(const struct hsm_action_item *hai, - char *buffer, int len) + char *buffer, size_t len) { - int i, sz, data_len; - char *ptr; - - ptr = buffer; - sz = len; - data_len = hai->hai_len - sizeof(*hai); - for (i = 0 ; (i < data_len) && (sz > 0) ; i++) - { - int cnt; - - cnt = snprintf(ptr, sz, "%.2X", - (unsigned char)hai->hai_data[i]); - ptr += cnt; - sz -= cnt; - } - *ptr = '\0'; - return buffer; + int i; + int data_len; + char *ptr; + + ptr = buffer; + data_len = hai->hai_len - sizeof(*hai); + for (i = 0; (i < data_len) && (len > 2); i++) { + snprintf(ptr, 3, "%02X", (unsigned char)hai->hai_data[i]); + ptr += 2; + len -= 2; + } + + *ptr = '\0'; + + return buffer; } /* Copytool action list */ -- 1.8.3.1