From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753629AbdJGWk2 (ORCPT ); Sat, 7 Oct 2017 18:40:28 -0400 Received: from fldsmtpe03.verizon.com ([140.108.26.142]:18276 "EHLO fldsmtpe03.verizon.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753746AbdJGWjJ (ORCPT ); Sat, 7 Oct 2017 18:39:09 -0400 From: "Levin, Alexander (Sasha Levin)" Cc: frank zago , James Simmons , "Greg Kroah-Hartman" , "Levin, Alexander (Sasha Levin)" X-Host: mariner.tdc.vzwcorp.com To: "linux-kernel@vger.kernel.org" , "stable@vger.kernel.org" Subject: [PATCH review for 3.18 09/12] staging: lustre: hsm: stack overrun in hai_dump_data_field Thread-Topic: [PATCH review for 3.18 09/12] staging: lustre: hsm: stack overrun in hai_dump_data_field Thread-Index: AQHTP7zu3+9KstNh+EKaRuEYARD8aQ== Date: Sat, 7 Oct 2017 22:38:01 +0000 Message-ID: <20171007223752.25085-9-alexander.levin@verizon.com> References: <20171007223752.25085-1-alexander.levin@verizon.com> In-Reply-To: <20171007223752.25085-1-alexander.levin@verizon.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ms-exchange-messagesentrepresentingtype: 1 x-ms-exchange-transport-fromentityheader: Hosted x-originating-ip: [10.144.60.250] Content-Type: text/plain; charset="iso-8859-1" MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by nfs id v97MeVCb026104 From: frank zago [ 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 Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-8171 Reviewed-on: http://review.whamcloud.com/20338 Reviewed-by: John L. Hammond Reviewed-by: Jean-Baptiste Riaux Reviewed-by: Oleg Drokin Signed-off-by: James Simmons Signed-off-by: Greg Kroah-Hartman Signed-off-by: Sasha Levin --- .../staging/lustre/lustre/include/lustre/lustre_user.h | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/drivers/staging/lustre/lustre/include/lustre/lustre_user.h b/drivers/staging/lustre/lustre/include/lustre/lustre_user.h index 89794fdfec9d..3b93270d5146 100644 --- 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; } -- 2.11.0