| From foo@baz Mon Nov 6 10:42:09 CET 2017 |
| From: frank zago <fzago@cray.com> |
| Date: Sat, 7 Oct 2017 22:38:01 +0000 |
| Subject: staging: lustre: hsm: stack overrun in hai_dump_data_field |
| |
| From: frank zago <fzago@cray.com> |
| |
| |
| [ Upstream commit 22aadb91c0a0055935109c175f5446abfb130702 ] |
| |
| The function hai_dump_data_field will do a stack buffer |
| overrun when cat'ing /sys/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. |
| |
| 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 <fzago@cray.com> |
| Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-8171 |
| Reviewed-on: http://review.whamcloud.com/20338 |
| Reviewed-by: John L. Hammond <john.hammond@intel.com> |
| Reviewed-by: Jean-Baptiste Riaux <riaux.jb@intel.com> |
| Reviewed-by: Oleg Drokin <oleg.drokin@intel.com> |
| Signed-off-by: James Simmons <jsimmons@infradead.org> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| Signed-off-by: Sasha Levin <alexander.levin@verizon.com> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| drivers/staging/lustre/lustre/include/lustre/lustre_user.h | 18 +++++-------- |
| 1 file changed, 8 insertions(+), 10 deletions(-) |
| |
| --- a/drivers/staging/lustre/lustre/include/lustre/lustre_user.h |
| +++ b/drivers/staging/lustre/lustre/include/lustre/lustre_user.h |
| @@ -1066,23 +1066,21 @@ struct hsm_action_item { |
| * \retval buffer |
| */ |
| static inline char *hai_dump_data_field(struct hsm_action_item *hai, |
| - char *buffer, int len) |
| + char *buffer, size_t len) |
| { |
| - int i, sz, data_len; |
| + int i, 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; |
| + 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; |
| } |
| |