From patchwork Sun Nov 20 01:08:20 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Venekamp X-Patchwork-Id: 1706683 X-Patchwork-Delegate: hauke@hauke-m.de Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=none (no SPF record) smtp.mailfrom=lists.openwrt.org (client-ip=2607:7c80:54:3::133; helo=bombadil.infradead.org; envelope-from=openwrt-devel-bounces+incoming=patchwork.ozlabs.org@lists.openwrt.org; receiver=) Authentication-Results: legolas.ozlabs.org; dkim=pass (2048-bit key; secure) header.d=lists.infradead.org header.i=@lists.infradead.org header.a=rsa-sha256 header.s=bombadil.20210309 header.b=U60ktD/2; dkim-atps=neutral Received: from bombadil.infradead.org (bombadil.infradead.org [IPv6:2607:7c80:54:3::133]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-384) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4NFCDy4vKRz23mg for ; Sun, 20 Nov 2022 12:12:50 +1100 (AEDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-Id:Date:Subject:To:From:Reply-To:Cc:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=cXSXBwkkO8phxmqjf6tR5kpgml+yX7EtzYodsTzg1n8=; b=U60ktD/2rCaqen zGyHqdX2C2MnNTvq143HVwPy4OyJpFvGXVO2U3S5LhO9YyRExhHMxoZEmQCHTOQgMc1rUJZP0x69z vRuwkQz8M5sXKvcwbLyd0h9Xd4vYfdTUr3mSfBd+H5bBLmQWsQSRrBlOFq8ZJcQ1lCF4cw2wrw7mp LQQHXQwFpoX9S2YTTwBn4200WhEhW0FmHXKHsbYPpJ10uniES1uU+Lx6/D2dfdNO35cLG5XCmj4Si uLE1lM48ECl2LgK6No8hMDXBC5teOX3A+TxoZOFmvKb881Qm5Nw/B1qDF0BAzlVG+FU0KyAUXYqQC QctvhBivJuncchqVhcew==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1owYph-000pC2-BY; Sun, 20 Nov 2022 01:09:17 +0000 Received: from virt1.bvwebdesign.nl ([149.210.228.112]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1owYp1-000p36-2x for openwrt-devel@lists.openwrt.org; Sun, 20 Nov 2022 01:08:38 +0000 Received: from localhost.localdomain (84-31-67-158.cable.dynamic.v4.ziggo.nl [84.31.67.158]) by virt1.bvwebdesign.nl (Postfix) with ESMTPSA id E26BBA5ED60 for ; Sun, 20 Nov 2022 02:08:28 +0100 (CET) From: Jan Venekamp To: openwrt-devel@lists.openwrt.org Subject: [PATCH v2 1/9] uci: fix use-after-free uci_set on update option Date: Sun, 20 Nov 2022 02:08:20 +0100 Message-Id: <20221120010828.23765-2-jan@venekamp.net> X-Mailer: git-send-email 2.32.0 (Apple Git-132) In-Reply-To: <20221120010828.23765-1-jan@venekamp.net> References: <20221120010828.23765-1-jan@venekamp.net> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20221119_170835_345308_E62A3363 X-CRM114-Status: UNSURE ( 9.56 ) X-CRM114-Notice: Please train this message. X-Spam-Score: -0.0 (/) X-Spam-Report: Spam detection software, running on the system "bombadil.infradead.org", has NOT identified this incoming email as spam. The original message has been attached to this so you can view it or label similar future email. If you have any questions, see the administrator of that system for details. Content preview: When uci_set is called with ptr->o set and ptr->option = NULL, then in uci_expand_ptr ptr->option is set to ptr->o->e.name. This will result in use-after-free because ptr->option is used in the call t [...] Content analysis details: (-0.0 points, 5.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 SPF_PASS SPF: sender matches SPF record -0.0 SPF_HELO_PASS SPF: HELO matches SPF record X-BeenThere: openwrt-devel@lists.openwrt.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: OpenWrt Development List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "openwrt-devel" Errors-To: openwrt-devel-bounces+incoming=patchwork.ozlabs.org@lists.openwrt.org When uci_set is called with ptr->o set and ptr->option = NULL, then in uci_expand_ptr ptr->option is set to ptr->o->e.name. This will result in use-after-free because ptr->option is used in the call to uci_add_delta after uci_free_option(ptr->o). Signed-off-by: Jan Venekamp --- list.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/list.c b/list.c index 24ed2ee..ac3686c 100644 --- a/list.c +++ b/list.c @@ -725,15 +725,16 @@ int uci_set(struct uci_context *ctx, struct uci_ptr *ptr) ptr->s = uci_alloc_section(ptr->p, ptr->value, ptr->section); ptr->last = &ptr->s->e; } else if (ptr->o && ptr->option) { /* update option */ - struct uci_option *o; + struct uci_option *old = ptr->o; if ((ptr->o->type == UCI_TYPE_STRING) && !strcmp(ptr->o->v.string, ptr->value)) return 0; - o = ptr->o; ptr->o = uci_alloc_option(ptr->s, ptr->option, ptr->value); - uci_free_option(o); + if (ptr->option == old->e.name) + ptr->option = ptr->o->e.name; + uci_free_option(old); ptr->last = &ptr->o->e; } else if (ptr->s && ptr->section) { /* update section */ char *s = uci_strdup(ctx, ptr->value); From patchwork Sun Nov 20 01:08:21 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Venekamp X-Patchwork-Id: 1706682 X-Patchwork-Delegate: hauke@hauke-m.de Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=none (no SPF record) smtp.mailfrom=lists.openwrt.org (client-ip=2607:7c80:54:3::133; helo=bombadil.infradead.org; envelope-from=openwrt-devel-bounces+incoming=patchwork.ozlabs.org@lists.openwrt.org; receiver=) Authentication-Results: legolas.ozlabs.org; dkim=pass (2048-bit key; secure) header.d=lists.infradead.org header.i=@lists.infradead.org header.a=rsa-sha256 header.s=bombadil.20210309 header.b=FyRaqNf2; dkim-atps=neutral Received: from bombadil.infradead.org (bombadil.infradead.org [IPv6:2607:7c80:54:3::133]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-384) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4NFCDk3Z0Dz23mg for ; Sun, 20 Nov 2022 12:12:38 +1100 (AEDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-Id:Date:Subject:To:From:Reply-To:Cc:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=26lmAn3dqELC6MO+g/1OP4yFatZbl/d8oMjgRFdbmck=; b=FyRaqNf2I2FMJd gPiTEFENRdL5WbaxGPsH/csb47m3jQYUZCHfgahqfu4ujFkm8MT7N0zo8IWsOE4eZHxrf2F5VAb0E XFjpck1skJFHuUvrtJWE91vKoncpNzEysPsjiGXk/xgZMuj7MFS9GKQCfT6QiHdvPeKMwKqFodIhs +PkGm7stTT8AuJBPuUEdZVRZZcs7bqwOTvm8jGbkn7+7hp6XZonp6SdRk6VmvkUciieu3Yq+N6MYG nt5boUzWIc1I0IwJWy6qiVsmHxzR6gT/wUlll0iHjnidPUKHhIYIX41q2QeOYMs+MCcuF62D5e5bc lWuOn2p59HS4IOnZ1KVw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1owYpJ-000p8U-96; Sun, 20 Nov 2022 01:08:53 +0000 Received: from virt1.bvwebdesign.nl ([149.210.228.112]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1owYp0-000p38-VW for openwrt-devel@lists.openwrt.org; Sun, 20 Nov 2022 01:08:36 +0000 Received: from localhost.localdomain (84-31-67-158.cable.dynamic.v4.ziggo.nl [84.31.67.158]) by virt1.bvwebdesign.nl (Postfix) with ESMTPSA id 00A08A5ED61 for ; Sun, 20 Nov 2022 02:08:28 +0100 (CET) From: Jan Venekamp To: openwrt-devel@lists.openwrt.org Subject: [PATCH v2 2/9] uci: maintain option position in uci_set Date: Sun, 20 Nov 2022 02:08:21 +0100 Message-Id: <20221120010828.23765-3-jan@venekamp.net> X-Mailer: git-send-email 2.32.0 (Apple Git-132) In-Reply-To: <20221120010828.23765-1-jan@venekamp.net> References: <20221120010828.23765-1-jan@venekamp.net> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20221119_170835_187207_A1B2E791 X-CRM114-Status: UNSURE ( 9.19 ) X-CRM114-Notice: Please train this message. X-Spam-Score: -0.0 (/) X-Spam-Report: Spam detection software, running on the system "bombadil.infradead.org", has NOT identified this incoming email as spam. The original message has been attached to this so you can view it or label similar future email. If you have any questions, see the administrator of that system for details. Content preview: Maintain the position of an option in the list when updating an option in uci_set. Signed-off-by: Jan Venekamp --- list.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) Content analysis details: (-0.0 points, 5.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 SPF_PASS SPF: sender matches SPF record -0.0 SPF_HELO_PASS SPF: HELO matches SPF record X-BeenThere: openwrt-devel@lists.openwrt.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: OpenWrt Development List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "openwrt-devel" Errors-To: openwrt-devel-bounces+incoming=patchwork.ozlabs.org@lists.openwrt.org Maintain the position of an option in the list when updating an option in uci_set. Signed-off-by: Jan Venekamp --- list.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/list.c b/list.c index ac3686c..a8f2a2c 100644 --- a/list.c +++ b/list.c @@ -76,7 +76,7 @@ uci_free_element(struct uci_element *e) } static struct uci_option * -uci_alloc_option(struct uci_section *s, const char *name, const char *value) +uci_alloc_option(struct uci_section *s, const char *name, const char *value, struct uci_list *after) { struct uci_package *p = s->package; struct uci_context *ctx = p->ctx; @@ -87,7 +87,7 @@ uci_alloc_option(struct uci_section *s, const char *name, const char *value) o->v.string = uci_dataptr(o); o->section = s; strcpy(o->v.string, value); - uci_list_add(&s->options, &o->e.list); + uci_list_insert(after ? after : s->options.prev, &o->e.list); return o; } @@ -719,7 +719,7 @@ int uci_set(struct uci_context *ctx, struct uci_ptr *ptr) return uci_delete(ctx, ptr); } else if (!ptr->o && ptr->option) { /* new option */ - ptr->o = uci_alloc_option(ptr->s, ptr->option, ptr->value); + ptr->o = uci_alloc_option(ptr->s, ptr->option, ptr->value, NULL); ptr->last = &ptr->o->e; } else if (!ptr->s && ptr->section) { /* new section */ ptr->s = uci_alloc_section(ptr->p, ptr->value, ptr->section); @@ -731,7 +731,7 @@ int uci_set(struct uci_context *ctx, struct uci_ptr *ptr) !strcmp(ptr->o->v.string, ptr->value)) return 0; - ptr->o = uci_alloc_option(ptr->s, ptr->option, ptr->value); + ptr->o = uci_alloc_option(ptr->s, ptr->option, ptr->value, &old->e.list); if (ptr->option == old->e.name) ptr->option = ptr->o->e.name; uci_free_option(old); From patchwork Sun Nov 20 01:08:22 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Venekamp X-Patchwork-Id: 1706679 X-Patchwork-Delegate: hauke@hauke-m.de Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=none (no SPF record) smtp.mailfrom=lists.openwrt.org (client-ip=2607:7c80:54:3::133; helo=bombadil.infradead.org; envelope-from=openwrt-devel-bounces+incoming=patchwork.ozlabs.org@lists.openwrt.org; receiver=) Authentication-Results: legolas.ozlabs.org; dkim=pass (2048-bit key; secure) header.d=lists.infradead.org header.i=@lists.infradead.org header.a=rsa-sha256 header.s=bombadil.20210309 header.b=kiOEZCwH; dkim-atps=neutral Received: from bombadil.infradead.org (bombadil.infradead.org [IPv6:2607:7c80:54:3::133]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-384) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4NFCDd3GZsz23nH for ; Sun, 20 Nov 2022 12:12:32 +1100 (AEDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-Id:Date:Subject:To:From:Reply-To:Cc:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=2fQZb6YaxtHY/TsdhvWtKVTACcjVmDdZJnUrPMmuOPA=; b=kiOEZCwHnEFLPm unlHYGY4hDf+YfF0CXpsjdMn/peYid0mPK/9jIBf520iWeJYdi0BRZVTIQh7npBDrpDPlHqGGb0ZK gloa/RyQmTcomgHL/molye6RGBAoDiFJ1W/6eTDEpD+4BxHuB9K5Ba+1fyvpNfcP3Gu32AJmetqfi P9gBSSV9GdU7h9yrq1ywFFYaBryr2V5ksbRTrGb/jN5QV4GMfWjpNY17r3hpXo6lHxmL+P6uCXYy9 3Mjk8Op19n1K9IF84OOdVyv0Wi2+M7qrAretAvBxPSXAVqGCDLJEPMLvv//NKKw0BwB0ZSc9tpUUu 71TIYj24dXwIWWuikrNQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1owYpt-000pCo-FL; Sun, 20 Nov 2022 01:09:29 +0000 Received: from virt1.bvwebdesign.nl ([149.210.228.112]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1owYp1-000p35-0p for openwrt-devel@lists.openwrt.org; Sun, 20 Nov 2022 01:08:38 +0000 Received: from localhost.localdomain (84-31-67-158.cable.dynamic.v4.ziggo.nl [84.31.67.158]) by virt1.bvwebdesign.nl (Postfix) with ESMTPSA id 128F9A5D2AB for ; Sun, 20 Nov 2022 02:08:29 +0100 (CET) From: Jan Venekamp To: openwrt-devel@lists.openwrt.org Subject: [PATCH v2 3/9] uci: optimize update option in uci_set Date: Sun, 20 Nov 2022 02:08:22 +0100 Message-Id: <20221120010828.23765-4-jan@venekamp.net> X-Mailer: git-send-email 2.32.0 (Apple Git-132) In-Reply-To: <20221120010828.23765-1-jan@venekamp.net> References: <20221120010828.23765-1-jan@venekamp.net> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20221119_170835_349161_34F0ECA4 X-CRM114-Status: GOOD ( 10.89 ) X-Spam-Score: -0.0 (/) X-Spam-Report: Spam detection software, running on the system "bombadil.infradead.org", has NOT identified this incoming email as spam. The original message has been attached to this so you can view it or label similar future email. If you have any questions, see the administrator of that system for details. Content preview: Optimize for the case when there is no need to reallocate memory when updating an option in uci_set. Signed-off-by: Jan Venekamp --- list.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) Content analysis details: (-0.0 points, 5.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 SPF_PASS SPF: sender matches SPF record -0.0 SPF_HELO_PASS SPF: HELO matches SPF record X-BeenThere: openwrt-devel@lists.openwrt.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: OpenWrt Development List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "openwrt-devel" Errors-To: openwrt-devel-bounces+incoming=patchwork.ozlabs.org@lists.openwrt.org Optimize for the case when there is no need to reallocate memory when updating an option in uci_set. Signed-off-by: Jan Venekamp --- list.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/list.c b/list.c index a8f2a2c..5148dfd 100644 --- a/list.c +++ b/list.c @@ -725,17 +725,19 @@ int uci_set(struct uci_context *ctx, struct uci_ptr *ptr) ptr->s = uci_alloc_section(ptr->p, ptr->value, ptr->section); ptr->last = &ptr->s->e; } else if (ptr->o && ptr->option) { /* update option */ - struct uci_option *old = ptr->o; - - if ((ptr->o->type == UCI_TYPE_STRING) && - !strcmp(ptr->o->v.string, ptr->value)) + if (ptr->o->type == UCI_TYPE_STRING && !strcmp(ptr->o->v.string, ptr->value)) return 0; - ptr->o = uci_alloc_option(ptr->s, ptr->option, ptr->value, &old->e.list); - if (ptr->option == old->e.name) - ptr->option = ptr->o->e.name; - uci_free_option(old); - ptr->last = &ptr->o->e; + if (ptr->o->type == UCI_TYPE_STRING && strlen(ptr->o->v.string) == strlen(ptr->value)) { + strcpy(ptr->o->v.string, ptr->value); + } else { + struct uci_option *old = ptr->o; + ptr->o = uci_alloc_option(ptr->s, ptr->option, ptr->value, &old->e.list); + if (ptr->option == old->e.name) + ptr->option = ptr->o->e.name; + uci_free_option(old); + ptr->last = &ptr->o->e; + } } else if (ptr->s && ptr->section) { /* update section */ char *s = uci_strdup(ctx, ptr->value); From patchwork Sun Nov 20 01:08:23 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Venekamp X-Patchwork-Id: 1706677 X-Patchwork-Delegate: hauke@hauke-m.de Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=none (no SPF record) smtp.mailfrom=lists.openwrt.org (client-ip=2607:7c80:54:3::133; helo=bombadil.infradead.org; envelope-from=openwrt-devel-bounces+incoming=patchwork.ozlabs.org@lists.openwrt.org; receiver=) Authentication-Results: legolas.ozlabs.org; dkim=pass (2048-bit key; secure) header.d=lists.infradead.org header.i=@lists.infradead.org header.a=rsa-sha256 header.s=bombadil.20210309 header.b=R9M82tAl; dkim-atps=neutral Received: from bombadil.infradead.org (bombadil.infradead.org [IPv6:2607:7c80:54:3::133]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-384) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4NFCDd3Ckdz23mg for ; Sun, 20 Nov 2022 12:12:31 +1100 (AEDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-Id:Date:Subject:To:From:Reply-To:Cc:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=nImTT44Xwl3ByIWN4wPfqQPHjqjbmL7vKoq9MeTUSEM=; b=R9M82tAlGA2MT0 fF4L9ZgaW3QBwWMZuL8tMzyInvF1IUs7PcI7zWMpDRqR2pr0wQZB+ArKmz4zEN3FyhpWOaek8z0TD 6jDCHEc3sjdkMnFo9HnmoU1WGPSe1KIruYSSStjrXtIGZ0OgbMQ9gyqMHjbrcSRp6J3ZSQ5PxPJ/1 yIaT48kFhd9gQeJanSNeA8ZPD9tiipIiDwunJAGCvUH1YdzwH3y4CYwYwxaph7Us5utPT7unNQ372 bpfV825qT171nR0IXs8lweTZiBiw4sfyRXAvu3u2bWSqRxjTaiTMjzRnY1Nd1Ev3J86qbrs85WM4S +addqhJZCbwAlxrz+2DA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1owYpV-000p9i-FT; Sun, 20 Nov 2022 01:09:05 +0000 Received: from virt1.bvwebdesign.nl ([149.210.228.112]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1owYp0-000p37-W9 for openwrt-devel@lists.openwrt.org; Sun, 20 Nov 2022 01:08:38 +0000 Received: from localhost.localdomain (84-31-67-158.cable.dynamic.v4.ziggo.nl [84.31.67.158]) by virt1.bvwebdesign.nl (Postfix) with ESMTPSA id 24824A5ED65 for ; Sun, 20 Nov 2022 02:08:29 +0100 (CET) From: Jan Venekamp To: openwrt-devel@lists.openwrt.org Subject: [PATCH v2 4/9] uci: fix use-after-free uci_add_list Date: Sun, 20 Nov 2022 02:08:23 +0100 Message-Id: <20221120010828.23765-5-jan@venekamp.net> X-Mailer: git-send-email 2.32.0 (Apple Git-132) In-Reply-To: <20221120010828.23765-1-jan@venekamp.net> References: <20221120010828.23765-1-jan@venekamp.net> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20221119_170835_205396_A575BB21 X-CRM114-Status: UNSURE ( 8.12 ) X-CRM114-Notice: Please train this message. X-Spam-Score: -0.0 (/) X-Spam-Report: Spam detection software, running on the system "bombadil.infradead.org", has NOT identified this incoming email as spam. The original message has been attached to this so you can view it or label similar future email. If you have any questions, see the administrator of that system for details. Content preview: When uci_add_list is called with ptr->o set and ptr->option = NULL, then in uci_expand_ptr ptr->option is set to ptr->o->e.name. If ptr->o->type is UCI_TYPE_STRING then prev is set to ptr->o. This wil [...] Content analysis details: (-0.0 points, 5.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 SPF_PASS SPF: sender matches SPF record -0.0 SPF_HELO_PASS SPF: HELO matches SPF record X-BeenThere: openwrt-devel@lists.openwrt.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: OpenWrt Development List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "openwrt-devel" Errors-To: openwrt-devel-bounces+incoming=patchwork.ozlabs.org@lists.openwrt.org When uci_add_list is called with ptr->o set and ptr->option = NULL, then in uci_expand_ptr ptr->option is set to ptr->o->e.name. If ptr->o->type is UCI_TYPE_STRING then prev is set to ptr->o. This will result in use-after-free because ptr->option is used in the call to uci_add_delta in uci_add_element_list after uci_free_option(prev). Signed-off-by: Jan Venekamp --- list.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/list.c b/list.c index 5148dfd..ba099b6 100644 --- a/list.c +++ b/list.c @@ -652,6 +652,8 @@ int uci_add_list(struct uci_context *ctx, struct uci_ptr *ptr) ptr->o = uci_alloc_list(ptr->s, ptr->option); if (prev) { uci_add_element_list(ctx, ptr, true); + if (ptr->option == prev->e.name) + ptr->option = ptr->o->e.name; uci_free_option(prev); ptr->value = value2; } From patchwork Sun Nov 20 01:08:24 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Venekamp X-Patchwork-Id: 1706681 X-Patchwork-Delegate: hauke@hauke-m.de Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=none (no SPF record) smtp.mailfrom=lists.openwrt.org (client-ip=2607:7c80:54:3::133; helo=bombadil.infradead.org; envelope-from=openwrt-devel-bounces+incoming=patchwork.ozlabs.org@lists.openwrt.org; receiver=) Authentication-Results: legolas.ozlabs.org; dkim=pass (2048-bit key; secure) header.d=lists.infradead.org header.i=@lists.infradead.org header.a=rsa-sha256 header.s=bombadil.20210309 header.b=TPqhQXP9; dkim-atps=neutral Received: from bombadil.infradead.org (bombadil.infradead.org [IPv6:2607:7c80:54:3::133]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-384) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4NFCDh4pkMz23mg for ; Sun, 20 Nov 2022 12:12:36 +1100 (AEDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-Id:Date:Subject:To:From:Reply-To:Cc:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=Y/ivmo7KYZUehGH0ffyfyYXuYAasZxfEWmiUh20wxfM=; b=TPqhQXP9erzzCb XnJDtQn2fLzCRRkwhs+5itHs/5IxyY+3+Ao84qTAr2Sl3Sbe2VIUX9O91ZScCud6Ex5TizWM8ZV1W 22pv9cbEz3G24E98WKmeHQCrDQYC/20ilnDx/CaoeyEql+3dKO9qeaXEW2eyH7PlKOi8p1iJopogS aehgNGWFREJHGvdgIY3WlBD4WdYy6S417isR8Xx+YuAEjLLVPpe+imm5MjIz77mq3oseCrun4ewpF 3o0z85fs68kimkIFswcyr2mApilZ667IUCe/PFLAxA/4KouulGK9UNtjLKyOv25RgIynpfl3dKKvc O8v+dw8x/MY8vy/R/tug==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1owYqf-000pPS-3r; Sun, 20 Nov 2022 01:10:17 +0000 Received: from virt1.bvwebdesign.nl ([149.210.228.112]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1owYp4-000p5W-5V for openwrt-devel@lists.openwrt.org; Sun, 20 Nov 2022 01:08:41 +0000 Received: from localhost.localdomain (84-31-67-158.cable.dynamic.v4.ziggo.nl [84.31.67.158]) by virt1.bvwebdesign.nl (Postfix) with ESMTPSA id 35B0FA5ED68 for ; Sun, 20 Nov 2022 02:08:29 +0100 (CET) From: Jan Venekamp To: openwrt-devel@lists.openwrt.org Subject: [PATCH v2 5/9] uci: fix atomicity of uci_add_list Date: Sun, 20 Nov 2022 02:08:24 +0100 Message-Id: <20221120010828.23765-6-jan@venekamp.net> X-Mailer: git-send-email 2.32.0 (Apple Git-132) In-Reply-To: <20221120010828.23765-1-jan@venekamp.net> References: <20221120010828.23765-1-jan@venekamp.net> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20221119_170838_621834_3DCF5C1B X-CRM114-Status: GOOD ( 12.69 ) X-Spam-Score: -0.0 (/) X-Spam-Report: Spam detection software, running on the system "bombadil.infradead.org", has NOT identified this incoming email as spam. The original message has been attached to this so you can view it or label similar future email. If you have any questions, see the administrator of that system for details. Content preview: The function uci_add_list is not atomic, when an alloc inside uci_add_element_list fails the option can be left in an indeterminate state. Refactor uci_add_list to fix this and make the code flow easier to read. Content analysis details: (-0.0 points, 5.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 SPF_PASS SPF: sender matches SPF record -0.0 SPF_HELO_PASS SPF: HELO matches SPF record X-BeenThere: openwrt-devel@lists.openwrt.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: OpenWrt Development List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "openwrt-devel" Errors-To: openwrt-devel-bounces+incoming=patchwork.ozlabs.org@lists.openwrt.org The function uci_add_list is not atomic, when an alloc inside uci_add_element_list fails the option can be left in an indeterminate state. Refactor uci_add_list to fix this and make the code flow easier to read. Signed-off-by: Jan Venekamp --- list.c | 74 +++++++++++++++++++++++++++++----------------------------- 1 file changed, 37 insertions(+), 37 deletions(-) diff --git a/list.c b/list.c index ba099b6..3518967 100644 --- a/list.c +++ b/list.c @@ -497,19 +497,6 @@ uci_expand_ptr(struct uci_context *ctx, struct uci_ptr *ptr, bool complete) return NULL; } -static void uci_add_element_list(struct uci_context *ctx, struct uci_ptr *ptr, bool internal) -{ - struct uci_element *e; - struct uci_package *p; - - p = ptr->p; - if (!internal && p->has_delta) - uci_add_delta(ctx, &p->delta, UCI_CMD_LIST_ADD, ptr->section, ptr->option, ptr->value); - - e = uci_alloc_generic(ctx, UCI_TYPE_ITEM, ptr->value, sizeof(struct uci_option)); - uci_list_add(&ptr->o->v.list, &e->list); -} - int uci_rename(struct uci_context *ctx, struct uci_ptr *ptr) { /* NB: UCI_INTERNAL use means without delta tracking */ @@ -623,8 +610,7 @@ int uci_add_list(struct uci_context *ctx, struct uci_ptr *ptr) { /* NB: UCI_INTERNAL use means without delta tracking */ bool internal = ctx && ctx->internal; - struct uci_option *volatile prev = NULL; - const char *volatile value2 = NULL; + struct uci_element *volatile e1 = NULL, *volatile e2 = NULL; UCI_HANDLE_ERR(ctx); @@ -632,34 +618,48 @@ int uci_add_list(struct uci_context *ctx, struct uci_ptr *ptr) UCI_ASSERT(ctx, ptr->s); UCI_ASSERT(ctx, ptr->value); - if (ptr->o) { - switch (ptr->o->type) { - case UCI_TYPE_STRING: - /* we already have a string value, convert that to a list */ - prev = ptr->o; - value2 = ptr->value; - ptr->value = ptr->o->v.string; - break; - case UCI_TYPE_LIST: - uci_add_element_list(ctx, ptr, internal); - return 0; - default: - UCI_THROW(ctx, UCI_ERR_INVAL); - break; - } + if (ptr->o && ptr->o->type != UCI_TYPE_LIST && ptr->o->type != UCI_TYPE_STRING) { + UCI_THROW(ctx, UCI_ERR_INVAL); } - ptr->o = uci_alloc_list(ptr->s, ptr->option); - if (prev) { - uci_add_element_list(ctx, ptr, true); - if (ptr->option == prev->e.name) + /* create new item */ + e1 = uci_alloc_generic(ctx, UCI_TYPE_ITEM, ptr->value, sizeof(struct uci_option)); + + if (!ptr->o) { + /* create new list */ + UCI_TRAP_SAVE(ctx, error); + ptr->o = uci_alloc_list(ptr->s, ptr->option); + UCI_TRAP_RESTORE(ctx); + ptr->last = &ptr->o->e; + } else if (ptr->o->type == UCI_TYPE_STRING) { + /* create new list and add old string value as item to list */ + struct uci_option *old = ptr->o; + UCI_TRAP_SAVE(ctx, error); + e2 = uci_alloc_generic(ctx, UCI_TYPE_ITEM, old->v.string, sizeof(struct uci_option)); + ptr->o = uci_alloc_list(ptr->s, ptr->option); + UCI_TRAP_RESTORE(ctx); + uci_list_add(&ptr->o->v.list, &e2->list); + + /* remove old option */ + if (ptr->option == old->e.name) ptr->option = ptr->o->e.name; - uci_free_option(prev); - ptr->value = value2; + uci_free_option(old); + ptr->last = &ptr->o->e; } - uci_add_element_list(ctx, ptr, internal); + + /* add new item to list */ + uci_list_add(&ptr->o->v.list, &e1->list); + + if (!internal && ptr->p->has_delta) + uci_add_delta(ctx, &ptr->p->delta, UCI_CMD_LIST_ADD, ptr->section, ptr->option, ptr->value); return 0; +error: + if (e1 != NULL) + uci_free_element(e1); + if (e2 != NULL) + uci_free_element(e2); + UCI_THROW(ctx, ctx->err); } int uci_del_list(struct uci_context *ctx, struct uci_ptr *ptr) From patchwork Sun Nov 20 01:08:25 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Venekamp X-Patchwork-Id: 1706676 X-Patchwork-Delegate: hauke@hauke-m.de Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=none (no SPF record) smtp.mailfrom=lists.openwrt.org (client-ip=2607:7c80:54:3::133; helo=bombadil.infradead.org; envelope-from=openwrt-devel-bounces+incoming=patchwork.ozlabs.org@lists.openwrt.org; receiver=) Authentication-Results: legolas.ozlabs.org; dkim=pass (2048-bit key; secure) header.d=lists.infradead.org header.i=@lists.infradead.org header.a=rsa-sha256 header.s=bombadil.20210309 header.b=ioHx8VVp; dkim-atps=neutral Received: from bombadil.infradead.org (bombadil.infradead.org [IPv6:2607:7c80:54:3::133]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-384) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4NFCDd3N3yz23nL for ; Sun, 20 Nov 2022 12:12:31 +1100 (AEDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-Id:Date:Subject:To:From:Reply-To:Cc:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=KdAG0qT3j1uqje7a727zMRXaZEd/a39ksP9OtL72/TI=; b=ioHx8VVpvvIcH9 4Eb/kNT0Q83XyB5FZGxV9dGsTMXVDJM9owq19/d3zG+y4an77goR7NpGMUj0QfOY+QcqX7pzvufN5 j5UlCOymFn23MTMii4uz+Hjn+egrpt/PRCnTVD8AJI2+NqLdIRVziwKHeJXUJLSrKdADLqooUjuRG rF5udfpn9wxKMM3na8sPW7/Rp0dJXO+Fcvssuc8PMrByEoCC74LqDks+VT9knn/9YqHAgdyIpc1X1 eAshUP+9E0VkPr8P/DyLcK3RMZGiQjj2WndputglbAxfOpPkv77B3ZuKslmjPr/X0zW04KWnJuZie mhOQ2fODH8XqcyrQYzXA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1owYq9-000pGV-KV; Sun, 20 Nov 2022 01:09:45 +0000 Received: from virt1.bvwebdesign.nl ([149.210.228.112]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1owYp4-000p5X-5T for openwrt-devel@lists.openwrt.org; Sun, 20 Nov 2022 01:08:39 +0000 Received: from localhost.localdomain (84-31-67-158.cable.dynamic.v4.ziggo.nl [84.31.67.158]) by virt1.bvwebdesign.nl (Postfix) with ESMTPSA id 48463A5ED62 for ; Sun, 20 Nov 2022 02:08:29 +0100 (CET) From: Jan Venekamp To: openwrt-devel@lists.openwrt.org Subject: [PATCH v2 6/9] uci: maintain option position in uci_add_list Date: Sun, 20 Nov 2022 02:08:25 +0100 Message-Id: <20221120010828.23765-7-jan@venekamp.net> X-Mailer: git-send-email 2.32.0 (Apple Git-132) In-Reply-To: <20221120010828.23765-1-jan@venekamp.net> References: <20221120010828.23765-1-jan@venekamp.net> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20221119_170838_424269_C01E4034 X-CRM114-Status: UNSURE ( 9.60 ) X-CRM114-Notice: Please train this message. X-Spam-Score: -0.0 (/) X-Spam-Report: Spam detection software, running on the system "bombadil.infradead.org", has NOT identified this incoming email as spam. The original message has been attached to this so you can view it or label similar future email. If you have any questions, see the administrator of that system for details. Content preview: Maintain the position of an option in the list when a string option is converted to a list option in uci_add_list. Signed-off-by: Jan Venekamp --- list.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) Content analysis details: (-0.0 points, 5.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 SPF_PASS SPF: sender matches SPF record -0.0 SPF_HELO_PASS SPF: HELO matches SPF record X-BeenThere: openwrt-devel@lists.openwrt.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: OpenWrt Development List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "openwrt-devel" Errors-To: openwrt-devel-bounces+incoming=patchwork.ozlabs.org@lists.openwrt.org Maintain the position of an option in the list when a string option is converted to a list option in uci_add_list. Signed-off-by: Jan Venekamp --- list.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/list.c b/list.c index 3518967..e6d631c 100644 --- a/list.c +++ b/list.c @@ -115,7 +115,7 @@ uci_free_option(struct uci_option *o) } static struct uci_option * -uci_alloc_list(struct uci_section *s, const char *name) +uci_alloc_list(struct uci_section *s, const char *name, struct uci_list *after) { struct uci_package *p = s->package; struct uci_context *ctx = p->ctx; @@ -125,7 +125,7 @@ uci_alloc_list(struct uci_section *s, const char *name) o->type = UCI_TYPE_LIST; o->section = s; uci_list_init(&o->v.list); - uci_list_add(&s->options, &o->e.list); + uci_list_insert(after ? after : s->options.prev, &o->e.list); return o; } @@ -628,7 +628,7 @@ int uci_add_list(struct uci_context *ctx, struct uci_ptr *ptr) if (!ptr->o) { /* create new list */ UCI_TRAP_SAVE(ctx, error); - ptr->o = uci_alloc_list(ptr->s, ptr->option); + ptr->o = uci_alloc_list(ptr->s, ptr->option, NULL); UCI_TRAP_RESTORE(ctx); ptr->last = &ptr->o->e; } else if (ptr->o->type == UCI_TYPE_STRING) { @@ -636,7 +636,7 @@ int uci_add_list(struct uci_context *ctx, struct uci_ptr *ptr) struct uci_option *old = ptr->o; UCI_TRAP_SAVE(ctx, error); e2 = uci_alloc_generic(ctx, UCI_TYPE_ITEM, old->v.string, sizeof(struct uci_option)); - ptr->o = uci_alloc_list(ptr->s, ptr->option); + ptr->o = uci_alloc_list(ptr->s, ptr->option, &old->e.list); UCI_TRAP_RESTORE(ctx); uci_list_add(&ptr->o->v.list, &e2->list); From patchwork Sun Nov 20 01:08:26 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Venekamp X-Patchwork-Id: 1706685 X-Patchwork-Delegate: hauke@hauke-m.de Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=none (no SPF record) smtp.mailfrom=lists.openwrt.org (client-ip=2607:7c80:54:3::133; helo=bombadil.infradead.org; envelope-from=openwrt-devel-bounces+incoming=patchwork.ozlabs.org@lists.openwrt.org; receiver=) Authentication-Results: legolas.ozlabs.org; dkim=pass (2048-bit key; secure) header.d=lists.infradead.org header.i=@lists.infradead.org header.a=rsa-sha256 header.s=bombadil.20210309 header.b=1cd7ZhPd; dkim-atps=neutral Received: from bombadil.infradead.org (bombadil.infradead.org [IPv6:2607:7c80:54:3::133]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-384) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4NFCGw6C0bz23mg for ; Sun, 20 Nov 2022 12:14:32 +1100 (AEDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-Id:Date:Subject:To:From:Reply-To:Cc:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=IJOI4uqZmpLZRFQzPksR2vs0eyrH5Fl43+zUm6mRlzg=; b=1cd7ZhPdC8HGWo auSdREi3LwBQrun2gpe3Kh9M0K+Jrq1UQX++SIAZMebRQdK0Idtxz4hMU04yL0Pcs0//m4Kv+bMD5 /wjuHIfEDlbJm5WUZnSMGbE9e3ot7L843dYW2XqOorP4Fe5epqEftaw8Ny94RYOiOY2lIUp++jxdK 8gxHzOmlsJpiKmUdPPOTu/bevXspACA08s3p8ZM7Ef3nvIGC1l57hmie7o8Qx8K/eff/8EqKdQ9sm AdrDWm98J1AMbdnV9hRECCcJsLbpJb+jmhXFGfqyjI8OObpBLa1T8cU//2QwPQpjUBhksHyes1J6h RbQk3MQco7zoRgc1gThQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1owYsB-000q1J-S2; Sun, 20 Nov 2022 01:11:52 +0000 Received: from virt1.bvwebdesign.nl ([149.210.228.112]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1owYp4-000p5Y-6H for openwrt-devel@lists.openwrt.org; Sun, 20 Nov 2022 01:08:41 +0000 Received: from localhost.localdomain (84-31-67-158.cable.dynamic.v4.ziggo.nl [84.31.67.158]) by virt1.bvwebdesign.nl (Postfix) with ESMTPSA id 5A86CA5ED6C for ; Sun, 20 Nov 2022 02:08:29 +0100 (CET) From: Jan Venekamp To: openwrt-devel@lists.openwrt.org Subject: [PATCH v2 7/9] uci: fix memory leak uci_set on update section Date: Sun, 20 Nov 2022 02:08:26 +0100 Message-Id: <20221120010828.23765-8-jan@venekamp.net> X-Mailer: git-send-email 2.32.0 (Apple Git-132) In-Reply-To: <20221120010828.23765-1-jan@venekamp.net> References: <20221120010828.23765-1-jan@venekamp.net> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20221119_170838_629011_F13B190C X-CRM114-Status: GOOD ( 18.79 ) X-Spam-Score: -0.0 (/) X-Spam-Report: Spam detection software, running on the system "bombadil.infradead.org", has NOT identified this incoming email as spam. The original message has been attached to this so you can view it or label similar future email. If you have any questions, see the administrator of that system for details. Content preview: If uci_realloc fails when updating a section in uci_set the reference to the memory allocated by s = uci_strdup() is lost. Also, if uci_strdup and uci_realloc both succeed it could happen that ptr->s->type == uci_dataptr(ptr->s) by accident. Then later on in uci_free_section the allocated ptr->s->type is not freed. Content analysis details: (-0.0 points, 5.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 SPF_PASS SPF: sender matches SPF record -0.0 SPF_HELO_PASS SPF: HELO matches SPF record X-BeenThere: openwrt-devel@lists.openwrt.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: OpenWrt Development List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "openwrt-devel" Errors-To: openwrt-devel-bounces+incoming=patchwork.ozlabs.org@lists.openwrt.org If uci_realloc fails when updating a section in uci_set the reference to the memory allocated by s = uci_strdup() is lost. Also, if uci_strdup and uci_realloc both succeed it could happen that ptr->s->type == uci_dataptr(ptr->s) by accident. Then later on in uci_free_section the allocated ptr->s->type is not freed. In order to fix this, instead of splitting the allocation of the the section and the type string, we create a new section with in-section storage to replace the old one. This also brings the code for updating a section more in line with the simular code for updating an option. Signed-off-by: Jan Venekamp --- list.c | 62 +++++++++++++++++++--------------------------------------- 1 file changed, 20 insertions(+), 42 deletions(-) diff --git a/list.c b/list.c index e6d631c..3e8a87c 100644 --- a/list.c +++ b/list.c @@ -30,12 +30,6 @@ static bool uci_list_set_pos(struct uci_list *head, struct uci_list *ptr, int po return (old_head != new_head); } -static inline void uci_list_fixup(struct uci_list *ptr) -{ - ptr->prev->next = ptr; - ptr->next->prev = ptr; -} - /* * uci_alloc_generic allocates a new uci_element with payload * payload is appended to the struct to save memory and reduce fragmentation @@ -182,34 +176,26 @@ static void uci_fixup_section(struct uci_context *ctx, struct uci_section *s) s->e.name = uci_strdup(ctx, buf); } -/* fix up option list HEAD pointers and pointer to section in options */ -static void uci_section_fixup_options(struct uci_section *s, bool no_options) +/* transfer options between two sections */ +static void uci_section_transfer_options(struct uci_section *dst, struct uci_section *src) { struct uci_element *e; - if (no_options) { - /* - * enforce empty list pointer state (s->next == s) when original - * section had no options in the first place - */ - uci_list_init(&s->options); - return; - } - - /* fix pointers to HEAD at end/beginning of list */ - uci_list_fixup(&s->options); + /* transfer the option list by inserting the new list HEAD and removing the old */ + uci_list_insert(&src->options, &dst->options); + uci_list_del(&src->options); - /* fix back pointer to section in options */ - uci_foreach_element(&s->options, e) { + /* update pointer to section in options */ + uci_foreach_element(&dst->options, e) { struct uci_option *o; o = uci_to_option(e); - o->section = s; + o->section = dst; } } static struct uci_section * -uci_alloc_section(struct uci_package *p, const char *type, const char *name) +uci_alloc_section(struct uci_package *p, const char *type, const char *name, struct uci_list *after) { struct uci_context *ctx = p->ctx; struct uci_section *s; @@ -226,7 +212,7 @@ uci_alloc_section(struct uci_package *p, const char *type, const char *name) s->anonymous = true; p->n_section++; - uci_list_add(&p->sections, &s->e.list); + uci_list_insert(after ? after : p->sections.prev, &s->e.list); return s; } @@ -551,7 +537,7 @@ int uci_add_section(struct uci_context *ctx, struct uci_package *p, const char * UCI_HANDLE_ERR(ctx); UCI_ASSERT(ctx, p != NULL); - s = uci_alloc_section(p, type, NULL); + s = uci_alloc_section(p, type, NULL, NULL); if (s && s->anonymous) uci_fixup_section(ctx, s); *res = s; @@ -724,7 +710,7 @@ int uci_set(struct uci_context *ctx, struct uci_ptr *ptr) ptr->o = uci_alloc_option(ptr->s, ptr->option, ptr->value, NULL); ptr->last = &ptr->o->e; } else if (!ptr->s && ptr->section) { /* new section */ - ptr->s = uci_alloc_section(ptr->p, ptr->value, ptr->section); + ptr->s = uci_alloc_section(ptr->p, ptr->value, ptr->section, NULL); ptr->last = &ptr->s->e; } else if (ptr->o && ptr->option) { /* update option */ if (ptr->o->type == UCI_TYPE_STRING && !strcmp(ptr->o->v.string, ptr->value)) @@ -741,22 +727,14 @@ int uci_set(struct uci_context *ctx, struct uci_ptr *ptr) ptr->last = &ptr->o->e; } } else if (ptr->s && ptr->section) { /* update section */ - char *s = uci_strdup(ctx, ptr->value); - - if (ptr->s->type == uci_dataptr(ptr->s)) { - /* drop the in-section storage of type name */ - bool no_options; - - no_options = uci_list_empty(&ptr->s->options); - ptr->last = NULL; - ptr->last = uci_realloc(ctx, ptr->s, sizeof(struct uci_section)); - ptr->s = uci_to_section(ptr->last); - uci_list_fixup(&ptr->s->e.list); - uci_section_fixup_options(ptr->s, no_options); - } else { - free(ptr->s->type); - } - ptr->s->type = s; + struct uci_section *old = ptr->s; + ptr->s = uci_alloc_section(ptr->p, ptr->value, old->e.name, &old->e.list); + uci_section_transfer_options(ptr->s, old); + if (ptr->section == old->e.name) + ptr->section = ptr->s->e.name; + uci_free_section(old); + ptr->s->package->n_section--; + ptr->last = &ptr->s->e; } else { UCI_THROW(ctx, UCI_ERR_INVAL); } From patchwork Sun Nov 20 01:08:27 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Venekamp X-Patchwork-Id: 1706680 X-Patchwork-Delegate: hauke@hauke-m.de Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=none (no SPF record) smtp.mailfrom=lists.openwrt.org (client-ip=2607:7c80:54:3::133; helo=bombadil.infradead.org; envelope-from=openwrt-devel-bounces+incoming=patchwork.ozlabs.org@lists.openwrt.org; receiver=) Authentication-Results: legolas.ozlabs.org; dkim=pass (2048-bit key; secure) header.d=lists.infradead.org header.i=@lists.infradead.org header.a=rsa-sha256 header.s=bombadil.20210309 header.b=07t5Ywam; dkim-atps=neutral Received: from bombadil.infradead.org (bombadil.infradead.org [IPv6:2607:7c80:54:3::133]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-384) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4NFCDg28DKz23nM for ; Sun, 20 Nov 2022 12:12:35 +1100 (AEDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-Id:Date:Subject:To:From:Reply-To:Cc:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=KpyWnFsWMzSqbkzFMw9GiSreDNGVRE8yKTQ1lHvdI4E=; b=07t5Ywam2MajEA 8f6E81g7Q0Hid7N0lSOCKcoPZ0LUey4z6y29FkdlfqLRE3roCKH0Tm0WrwQHBNflaXzgZxs4Eby6B o6w/TwigLKRtxskQ1Lh948PPkMkKgY77fWrLQSc2Rv3utNamHnAGhz/u2sM3yVvVVuWx2Lm8ivhRy KU8/7Iz64tALcceL3BryVbjkaNkWzMLGnpTzV57lUUTP9nw+zSXG3Ue20fZLWU7P0TF05Mrot29fl RUY/c0xKhUievNOJQRAvPDSEG94csiHqsOzf9bp8QiOOzryrEQlCBpScTAYyPo6WEhrzqNlj5YkZt GsFB18q6/0Fo0Nn0ndiQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1owYqN-000pK1-7c; Sun, 20 Nov 2022 01:09:59 +0000 Received: from virt1.bvwebdesign.nl ([149.210.228.112]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1owYp4-000p5Z-65 for openwrt-devel@lists.openwrt.org; Sun, 20 Nov 2022 01:08:39 +0000 Received: from localhost.localdomain (84-31-67-158.cable.dynamic.v4.ziggo.nl [84.31.67.158]) by virt1.bvwebdesign.nl (Postfix) with ESMTPSA id 6ED1EA5ED6B for ; Sun, 20 Nov 2022 02:08:29 +0100 (CET) From: Jan Venekamp To: openwrt-devel@lists.openwrt.org Subject: [PATCH v2 8/9] uci: optimize update section in uci_set Date: Sun, 20 Nov 2022 02:08:27 +0100 Message-Id: <20221120010828.23765-9-jan@venekamp.net> X-Mailer: git-send-email 2.32.0 (Apple Git-132) In-Reply-To: <20221120010828.23765-1-jan@venekamp.net> References: <20221120010828.23765-1-jan@venekamp.net> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20221119_170838_429870_45F1D370 X-CRM114-Status: GOOD ( 11.15 ) X-Spam-Score: -0.0 (/) X-Spam-Report: Spam detection software, running on the system "bombadil.infradead.org", has NOT identified this incoming email as spam. The original message has been attached to this so you can view it or label similar future email. If you have any questions, see the administrator of that system for details. Content preview: Optimize for the case when there is no need to update the section and the case there is no need to reallocate memory when updating a section in uci_set. Signed-off-by: Jan Venekamp --- list.c | 23 +++++++++++++++-------- tests/shunit2/tests.d/090_cli_options | 8 +++++--- 2 files changed, 20 insertions(+), 11 deletions(-) Content analysis details: (-0.0 points, 5.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 SPF_PASS SPF: sender matches SPF record -0.0 SPF_HELO_PASS SPF: HELO matches SPF record X-BeenThere: openwrt-devel@lists.openwrt.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: OpenWrt Development List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "openwrt-devel" Errors-To: openwrt-devel-bounces+incoming=patchwork.ozlabs.org@lists.openwrt.org Optimize for the case when there is no need to update the section and the case there is no need to reallocate memory when updating a section in uci_set. Signed-off-by: Jan Venekamp --- list.c | 23 +++++++++++++++-------- tests/shunit2/tests.d/090_cli_options | 8 +++++--- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/list.c b/list.c index 3e8a87c..1640213 100644 --- a/list.c +++ b/list.c @@ -727,14 +727,21 @@ int uci_set(struct uci_context *ctx, struct uci_ptr *ptr) ptr->last = &ptr->o->e; } } else if (ptr->s && ptr->section) { /* update section */ - struct uci_section *old = ptr->s; - ptr->s = uci_alloc_section(ptr->p, ptr->value, old->e.name, &old->e.list); - uci_section_transfer_options(ptr->s, old); - if (ptr->section == old->e.name) - ptr->section = ptr->s->e.name; - uci_free_section(old); - ptr->s->package->n_section--; - ptr->last = &ptr->s->e; + if (!strcmp(ptr->s->type, ptr->value)) + return 0; + + if (strlen(ptr->s->type) == strlen(ptr->value)) { + strcpy(ptr->s->type, ptr->value); + } else { + struct uci_section *old = ptr->s; + ptr->s = uci_alloc_section(ptr->p, ptr->value, old->e.name, &old->e.list); + uci_section_transfer_options(ptr->s, old); + if (ptr->section == old->e.name) + ptr->section = ptr->s->e.name; + uci_free_section(old); + ptr->s->package->n_section--; + ptr->last = &ptr->s->e; + } } else { UCI_THROW(ctx, UCI_ERR_INVAL); } diff --git a/tests/shunit2/tests.d/090_cli_options b/tests/shunit2/tests.d/090_cli_options index 55920a2..aff8316 100644 --- a/tests/shunit2/tests.d/090_cli_options +++ b/tests/shunit2/tests.d/090_cli_options @@ -11,8 +11,9 @@ test_add_delta() { # save new changes in "$new_savedir" mkdir -p "$new_savedir" touch "$new_savedir/delta" - $UCI -P "$new_savedir" set delta.sec0=sectype + $UCI -P "$new_savedir" set delta.sec0=tmptype $UCI -P "$new_savedir" add_list delta.sec0.li0=1 + $UCI -P "$new_savedir" set delta.sec0=sectype assertEquals "delta.sec0='sectype' delta.sec0.li0+='0'" "$($UCI changes)" @@ -22,8 +23,9 @@ delta.sec0.li0+='0'" "$($UCI changes)" assertTrue "$?" assertEquals "delta.sec0='sectype' delta.sec0.li0+='0' -delta.sec0='sectype' -delta.sec0.li0+='1'" "$cmdoutput" +delta.sec0='tmptype' +delta.sec0.li0+='1' +delta.sec0='sectype'" "$cmdoutput" # check combined export. Order matters here. cmdoutput="$($UCI -P "$new_savedir" export)" From patchwork Sun Nov 20 01:08:28 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Venekamp X-Patchwork-Id: 1706684 X-Patchwork-Delegate: hauke@hauke-m.de Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=none (no SPF record) smtp.mailfrom=lists.openwrt.org (client-ip=2607:7c80:54:3::133; helo=bombadil.infradead.org; envelope-from=openwrt-devel-bounces+incoming=patchwork.ozlabs.org@lists.openwrt.org; receiver=) Authentication-Results: legolas.ozlabs.org; dkim=pass (2048-bit key; secure) header.d=lists.infradead.org header.i=@lists.infradead.org header.a=rsa-sha256 header.s=bombadil.20210309 header.b=ChCoxcy1; dkim-atps=neutral Received: from bombadil.infradead.org (bombadil.infradead.org [IPv6:2607:7c80:54:3::133]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-384) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4NFCFV2mtcz23mg for ; Sun, 20 Nov 2022 12:13:18 +1100 (AEDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-Id:Date:Subject:To:From:Reply-To:Cc:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=UneRqs8QsBVQ765Lq19ehMP04VspNIgweTbJ5igZDDA=; b=ChCoxcy1TLDKYX 5hQT9byICvHTajW/71PzGjwtQztQP0T0BKw/zYIDJbF32umHDgHek5ZuSUjvIqwZsmME4Hj35f9Ut VP+l07IBH7Oqae4s99xCCZw7HPcV22pvl3QM6n9/bgaHQm5E4DdoThKA0kf+ukyU0gU30DYr9jTol hT+YbiDv2E9ix4IFRtv19LiaHZ2QCBZsfngRt9+xoAb7OjHlpLIdQZ8lphnb44rKzcMa/M4Hl/F9J YvANKcMvG5ooBM54hxfrXyHkFKzKzugE0/G490SayRGq8NgqQWJb490FpH5HZ5IFQ1w3FaLlCiFVq /l+dQ/pXXgn67ttIwBLw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1owYrC-000pcE-Gm; Sun, 20 Nov 2022 01:10:51 +0000 Received: from virt1.bvwebdesign.nl ([149.210.228.112]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1owYp5-000p5s-6H for openwrt-devel@lists.openwrt.org; Sun, 20 Nov 2022 01:08:41 +0000 Received: from localhost.localdomain (84-31-67-158.cable.dynamic.v4.ziggo.nl [84.31.67.158]) by virt1.bvwebdesign.nl (Postfix) with ESMTPSA id 80DEAA5ED76 for ; Sun, 20 Nov 2022 02:08:29 +0100 (CET) From: Jan Venekamp To: openwrt-devel@lists.openwrt.org Subject: [PATCH v2 9/9] uci: macro uci_alloc_element not in uci.h Date: Sun, 20 Nov 2022 02:08:28 +0100 Message-Id: <20221120010828.23765-10-jan@venekamp.net> X-Mailer: git-send-email 2.32.0 (Apple Git-132) In-Reply-To: <20221120010828.23765-1-jan@venekamp.net> References: <20221120010828.23765-1-jan@venekamp.net> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20221119_170839_381556_225B313D X-CRM114-Status: UNSURE ( 9.12 ) X-CRM114-Notice: Please train this message. X-Spam-Score: -0.0 (/) X-Spam-Report: Spam detection software, running on the system "bombadil.infradead.org", has NOT identified this incoming email as spam. The original message has been attached to this so you can view it or label similar future email. If you have any questions, see the administrator of that system for details. Content preview: The macro uci_alloc_element is in the public header file uci.h. However, the macros output refers to uci_alloc_generic wich is in uci_internal.h and not public. Thus, uci_alloc_element should be priva [...] Content analysis details: (-0.0 points, 5.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 SPF_PASS SPF: sender matches SPF record -0.0 SPF_HELO_PASS SPF: HELO matches SPF record X-BeenThere: openwrt-devel@lists.openwrt.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: OpenWrt Development List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "openwrt-devel" Errors-To: openwrt-devel-bounces+incoming=patchwork.ozlabs.org@lists.openwrt.org The macro uci_alloc_element is in the public header file uci.h. However, the macros output refers to uci_alloc_generic wich is in uci_internal.h and not public. Thus, uci_alloc_element should be private as well and moved to uci_internal.h. Signed-off-by: Jan Venekamp --- uci.h | 10 ---------- uci_internal.h | 3 +++ 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/uci.h b/uci.h index b385e2b..d0374f2 100644 --- a/uci.h +++ b/uci.h @@ -613,16 +613,6 @@ BUILD_CAST(option) #define uci_to_option(ptr) container_of(ptr, struct uci_option, e) #endif -/** - * uci_alloc_element: allocate a generic uci_element, reserve a buffer and typecast - * @ctx: uci context - * @type: {package,section,option} - * @name: string containing the name of the element - * @datasize: additional buffer size to reserve at the end of the struct - */ -#define uci_alloc_element(ctx, type, name, datasize) \ - uci_to_ ## type (uci_alloc_generic(ctx, uci_type_ ## type, name, sizeof(struct uci_ ## type) + datasize)) - #define uci_dataptr(ptr) \ (((char *) ptr) + sizeof(*ptr)) diff --git a/uci_internal.h b/uci_internal.h index 34528f0..ff4ee8c 100644 --- a/uci_internal.h +++ b/uci_internal.h @@ -42,6 +42,9 @@ struct uci_parse_context #define pctx_char(pctx, i) ((pctx)->buf[(i)]) #define pctx_cur_char(pctx) pctx_char(pctx, pctx_pos(pctx)) +#define uci_alloc_element(ctx, type, name, datasize) \ + uci_to_ ## type (uci_alloc_generic(ctx, uci_type_ ## type, name, sizeof(struct uci_ ## type) + datasize)) + extern const char *uci_confdir; extern const char *uci_savedir;