Re: [dpdk-dev] [PATCH] fixed buffer overrun in handling log messages

[Olivier MATZ <olivier.matz@6wind.com>]

[-- Attachment #1: Type: text/plain, Size: 1470 bytes --]

Hi,

Thank you for this patch. You are right, the '\0' is written
outside the bounds of the buffer. I would suggest a minor modification
to your patch, please see attachment.

Regards,
Olivier


On 05/01/2013 08:50 PM, Han, Dongsu wrote:
>   I'm sending a proposed patch to fix the buffer overrun problem in
> handling log messages.
>
> Dongsu Han
>
>
> _______________________________________________
> dev mailing list
> dev@dpdk.org
> http://dpdk.org/ml/listinfo/dev


-- 
Olivier MATZ
Tel +33-1-39-30-92-57
www.6wind.com
================================================================================
This e-mail message, including any attachments, is for the sole use of 
the intended recipient(s) and contains information that is confidential 
and proprietary to 6WIND. All unauthorized review, use, disclosure or 
distribution is prohibited. If you are not the intended recipient, 
please contact the sender by reply e-mail and destroy all copies of the 
original message.
Ce courriel ainsi que toutes les pièces jointes, est uniquement destiné 
à son ou ses destinataires. Il contient des informations confidentielles 
qui sont la propriété de 6WIND. Toute révélation, distribution ou copie 
des informations qu'il contient est strictement interdite. Si vous avez 
reçu ce message par erreur, veuillez immédiatement le signaler à 
l'émetteur et détruire toutes les données reçues
================================================================================

[-- Attachment #2: 0001-eal-log-fix-memory-corruption.patch --]
[-- Type: text/x-patch, Size: 1702 bytes --]

>From e14f9b05dbc8aca332fa6532382df6a70ff6da25 Mon Sep 17 00:00:00 2001
From: Dongsu Han <dongsuh@cs.cmu.edu>
Date: Thu, 2 May 2013 10:14:58 +0200
Subject: eal/log: fix memory corruption

The '\0' is written outside the bounds of the log buffer, which can
result in memory corruption or display issues with log messages.

Use a new constant LOG_BUF_SIZE to store the effective size of the
buffer in struct log_history.

Acked-by: Olivier Matz <olivier.matz@6wind.com>
Signed-off-by: Dongsu Han <dongsuh@cs.cmu.edu>
---
 lib/librte_eal/common/eal_common_log.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
index 1362109..21970c5 100644
--- a/lib/librte_eal/common/eal_common_log.c
+++ b/lib/librte_eal/common/eal_common_log.c
@@ -64,6 +64,7 @@
 #include "eal_private.h"
 
 #define LOG_ELT_SIZE     2048
+#define LOG_BUF_SIZE     (LOG_ELT_SIZE - sizeof(struct log_history))
 
 #define LOG_HISTORY_MP_NAME "log_history"
 
@@ -196,7 +197,7 @@ rte_log_add_in_history(const char *buf, size_t size)
 	}
 
 	/* not enough room for msg, buffer go back in mempool */
-	if (size >= (LOG_ELT_SIZE - sizeof(*hist_buf))) {
+	if (size >= LOG_BUF_SIZE) {
 		rte_mempool_mp_put(log_history_mp, hist_buf);
 		rte_spinlock_unlock(&log_list_lock);
 		return -ENOBUFS;
@@ -204,7 +205,7 @@ rte_log_add_in_history(const char *buf, size_t size)
 
 	/* add in history */
 	memcpy(hist_buf->buf, buf, size);
-	hist_buf->buf[LOG_ELT_SIZE-1] = '\0';
+	hist_buf->buf[LOG_BUF_SIZE-1] = '\0';
 	hist_buf->size = size;
 	STAILQ_INSERT_TAIL(&log_history, hist_buf, next);
 	rte_spinlock_unlock(&log_list_lock);
-- 
1.7.10.4