From patchwork Mon Mar 20 08:31:07 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alexandru Ardelean X-Patchwork-Id: 740838 X-Patchwork-Delegate: pablo@netfilter.org Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3vmq2C1prBz9s0m for ; Mon, 20 Mar 2017 19:32:35 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="ez1CbLT9"; dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753244AbdCTIcf (ORCPT ); Mon, 20 Mar 2017 04:32:35 -0400 Received: from mail-wm0-f65.google.com ([74.125.82.65]:35207 "EHLO mail-wm0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753064AbdCTIc0 (ORCPT ); Mon, 20 Mar 2017 04:32:26 -0400 Received: by mail-wm0-f65.google.com with SMTP id z133so12974004wmb.2 for ; Mon, 20 Mar 2017 01:32:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id; bh=/Xym5x8RQCRfc0cpg00CoB6pzhLZy+VH78XesyTOUhA=; b=ez1CbLT9eFHqFnngSe6UuHRZIQ8RPWwplFNAL/Hpc35E+fk1nz3GY0qof095sgp6EB 6Xga1ooqE9m9ucN1Ij7OVEzN7DTBIT2dUvDcUMR0DUSCVgF0Vo/g6oIf8XqUL6n49VQH NvqdJZTj9O6iSRJlauxVWMVcpzyGMfd9za3+FkxKUutrbbU/C1t9dQ8Ex0fgoH4e5RN6 j5FHD+N2pzxaMMb05Kz7rfzbR06/OFkkhLx4WhvH/SNwET6cmv4SW31/CyrwgwKDx0XA FneMjgMRL+J0/YVn3+LqGQJJ7ZnX/8KB/09OL62sRuI6q5U1D0tvz7SRnaJ92l45LhLd eOLQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=/Xym5x8RQCRfc0cpg00CoB6pzhLZy+VH78XesyTOUhA=; b=piyl5R1n9NACaSS9OGOnRvw8+muylqnYzOFjWsUQPaXEAi6OvbKrVPGjVKXaAaTF0G KnkQLGaTKGla0xXw5TSgXwgEVwPm9A/H1xtuQvzGX13Vwgoi0GFCYw/nvBJMIv6z/Ozu TDu2miTZEAuhhnX4Nge3NDnE3rmtPXSEj+BUIUAMEFyB7i5r6V3kWzi6MNsF76Z9LCjH oZ5Cn26yr9iQBoWN1GPgpmFs+Rw+TZsGLP5W4my8ToGLXO2vnuBvv/NN1xjjPNASSE/7 bNn59GKMwT1uXyQhELcEf73CuIKZbGmbQVasKU5vKA1vpHfdWSTCoYlvS9HiV4qi6mUi aCLw== X-Gm-Message-State: AFeK/H2OxAT7jPH/u+CtaLVY8gNHxRKeW/QdKD2j+EEtPjb6gF4I4UeZBREut2By84rntQ== X-Received: by 10.28.165.147 with SMTP id o141mr9042909wme.67.1489998744849; Mon, 20 Mar 2017 01:32:24 -0700 (PDT) Received: from localhost.localdomain ([5.2.198.78]) by smtp.googlemail.com with ESMTPSA id k190sm6877091wmg.10.2017.03.20.01.32.22 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Mon, 20 Mar 2017 01:32:23 -0700 (PDT) From: Alexandru Ardelean To: netfilter-devel@vger.kernel.org Cc: eric@regit.org, Alexandru Ardelean Subject: [PATCH] ulogd: add +1 char for null char Date: Mon, 20 Mar 2017 10:31:07 +0200 Message-Id: <1489998667-15183-1-git-send-email-ardeleanalex@gmail.com> X-Mailer: git-send-email 2.7.4 Sender: netfilter-devel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netfilter-devel@vger.kernel.org This is a bit zealous to fix like this, but it seems to work. The crash was reproduced on ppc32, with GCC 5.4 & musl libc 1.1.16. And also on LEDE (mips_24kc and ARM): https://github.com/openwrt/packages/issues/4123 https://github.com/openwrt/packages/issues/4090 I personally saw it on ppc32. The offending code was in `pluginstance_alloc_init()` line 671: ``` memcpy(pi->id, pi_id, sizeof(pi->id)); ``` Seems that it would copy 1 char from the stack, and that caused some failsafes to kick in. This fix addresses the issue directly. Maybe a more appropriate rework of string stuff would be needed. What I also noticed, is that there's also places in the code that define name[ULOGD_MAX_KEYLEN+1] and some that don't add the +1 char. Basically, this just aligns the remaining bits of code that don't add the +1 char. Signed-off-by: Alexandru Ardelean --- output/sqlite3/ulogd_output_SQLITE3.c | 6 +++--- src/ulogd.c | 2 +- util/db.c | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/output/sqlite3/ulogd_output_SQLITE3.c b/output/sqlite3/ulogd_output_SQLITE3.c index 20ceb3b..ea66061 100644 --- a/output/sqlite3/ulogd_output_SQLITE3.c +++ b/output/sqlite3/ulogd_output_SQLITE3.c @@ -48,7 +48,7 @@ struct field { TAILQ_ENTRY(field) link; - char name[ULOGD_MAX_KEYLEN]; + char name[ULOGD_MAX_KEYLEN+1]; /* +1 for null char */ struct ulogd_key *key; }; @@ -214,7 +214,7 @@ sqlite3_createstmt(struct ulogd_pluginstance *pi) { struct sqlite3_priv *priv = (void *)pi->private; struct field *f; - char buf[ULOGD_MAX_KEYLEN]; + char buf[ULOGD_MAX_KEYLEN+1]; /* +1 for null char */ char *underscore; char *stmt_pos; int i, cols = 0; @@ -305,7 +305,7 @@ static int sqlite3_init_db(struct ulogd_pluginstance *pi) { struct sqlite3_priv *priv = (void *)pi->private; - char buf[ULOGD_MAX_KEYLEN]; + char buf[ULOGD_MAX_KEYLEN+1]; char *underscore; struct field *f; sqlite3_stmt *schema_stmt; diff --git a/src/ulogd.c b/src/ulogd.c index 5b9a586..0d6a367 100644 --- a/src/ulogd.c +++ b/src/ulogd.c @@ -942,7 +942,7 @@ static int create_stack(const char *option) /* PASS 1: find and instanciate plugins of stack, link them together */ for (tok = strtok(buf, ",\n"); tok; tok = strtok(NULL, ",\n")) { char *plname, *equals; - char pi_id[ULOGD_MAX_KEYLEN]; + char pi_id[ULOGD_MAX_KEYLEN+1]; /* +1 for the null char */ struct ulogd_pluginstance *pi; struct ulogd_plugin *pl; diff --git a/util/db.c b/util/db.c index c9aec41..6af4555 100644 --- a/util/db.c +++ b/util/db.c @@ -96,7 +96,7 @@ static int sql_createstmt(struct ulogd_pluginstance *upi) if (strncasecmp(procedure,"INSERT", strlen("INSERT")) == 0 && (procedure[strlen("INSERT")] == '\0' || procedure[strlen("INSERT")] == ' ')) { - char buf[ULOGD_MAX_KEYLEN]; + char buf[ULOGD_MAX_KEYLEN+1]; /* +1 for null char */ char *underscore; if(procedure[6] == '\0') {