From patchwork Tue Aug 16 11:23:50 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Venekamp X-Patchwork-Id: 1666799 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=s6+XOHsa; 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 4M6TP01xDzz1ygF for ; Tue, 16 Aug 2022 21:26:12 +1000 (AEST) 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=s6+XOHsa1483fa zUsxR784wbLZ4QtcNYSz24fyW77hlcMuaPKR8eXNBJGRDH3RI06O6RRQ63WnOqnzzwk3/PNhgjiWN DxS6e3sBPqyA8l/UChJXEZUSWfwXqMQ7glZLrn7XQdLKbqddSJN8c0HH/j8GflKnJdozceOc8AsZk 4bNg8h4Zw8iCJffwMJgCg88wJ3Pfu3yBDsAuRmzGoUxbgJUyLZZlSgg3wyWBsFF0Kmex6A5GIW+2Q fVP7I91MRRFUV+mOwvxh+N7H5F0b+jFXvo+rSBuwr2WoMYWL7s7LdHWJlJlYwfvqYQKBXpAABd2CJ cTR0OUhgGNSHwlF/uVDw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1oNugL-001WG2-K7; Tue, 16 Aug 2022 11:24:25 +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 1oNug2-001W1j-J6 for openwrt-devel@lists.openwrt.org; Tue, 16 Aug 2022 11:24:08 +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 5D6E191DB8E for ; Tue, 16 Aug 2022 13:23:59 +0200 (CEST) From: Jan Venekamp To: openwrt-devel@lists.openwrt.org Subject: [PATCH 1/9] uci: fix use-after-free uci_set on update option Date: Tue, 16 Aug 2022 13:23:50 +0200 Message-Id: <20220816112358.75801-2-jan@venekamp.net> X-Mailer: git-send-email 2.32.0 (Apple Git-132) In-Reply-To: <20220816112358.75801-1-jan@venekamp.net> References: <20220816112358.75801-1-jan@venekamp.net> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220816_042406_824300_6752FA53 X-CRM114-Status: UNSURE ( 9.81 ) 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 Tue Aug 16 11:23:51 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Venekamp X-Patchwork-Id: 1666802 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=2DRQfuAr; 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 4M6TPr2nPQz1ygF for ; Tue, 16 Aug 2022 21:26:56 +1000 (AEST) 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=2DRQfuArQLx3dJ YDsaDPe9zuy8pdkwlH8wMRxWhywObLsp1ZCOE48LvkWQVZRBeYdZyvA+acr5mR57GZjD8WltvBzkE /DQ7R6Kds6CWxhudczSPof4mxY4bwkIc+6baRk8fhQDvakvs4e3cqKb20cQn8tSH/yivjJ5R9ERcf QlVAPQaUwF9J6PMlGaPgmi9xAxW6nl9hzoa6DUIOkVDmIX3fHMeO+4vN1+YkOPsGUie9uUhVHbo1a V245GaaJEo1zhG/JFmEyqGiE+mm/v8Qc0zJaTEhnxZ+vJMCGicAFYx54ipa3UI/BE9snwwnW2UU2N OgZlKRT5RzjiqEmzleSg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1oNuh7-001WgH-6k; Tue, 16 Aug 2022 11:25:13 +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 1oNug2-001W1i-I3 for openwrt-devel@lists.openwrt.org; Tue, 16 Aug 2022 11:24:09 +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 6F9CF84F433 for ; Tue, 16 Aug 2022 13:23:59 +0200 (CEST) From: Jan Venekamp To: openwrt-devel@lists.openwrt.org Subject: [PATCH 2/9] uci: maintain option position in uci_set Date: Tue, 16 Aug 2022 13:23:51 +0200 Message-Id: <20220816112358.75801-3-jan@venekamp.net> X-Mailer: git-send-email 2.32.0 (Apple Git-132) In-Reply-To: <20220816112358.75801-1-jan@venekamp.net> References: <20220816112358.75801-1-jan@venekamp.net> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220816_042406_812566_FDBC5FC6 X-CRM114-Status: UNSURE ( 9.33 ) 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 Tue Aug 16 11:23:52 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Venekamp X-Patchwork-Id: 1666800 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=aimymdEi; 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 4M6TP13Ckyz1ygF for ; Tue, 16 Aug 2022 21:26:13 +1000 (AEST) 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=aimymdEiqolTwM EzAmPPfI5OUldyjkddh10iMzfW+S5sQ2KDTmXpE/bwwoG8Uhr/1BcR7t4GYTE/6+cdHqylgNZEr0x MJDypt+tApT/EyUAM03QNgBHhqa8I541L2jAwSAH+6wwk1q7b1y1kavC/4bP71nLcd4ih2YiMhfiI 8QdUIyjXKd6Pc8GqbpmEFORB4VzCQTOfYCfgx5C0YDITk5S7I699uEbOHAQxDdKpYwZM+TaIyTpLO bAFDEpcvU4AWtsZIrNvTwHlYYBPdGs815Em4fXkkxJndz9oenPHrkLAxhbFie9C+jWPPYD3VAiYMQ ZaPisXGuqQ/sGRovasBA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1oNugb-001WML-0Y; Tue, 16 Aug 2022 11:24:41 +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 1oNug2-001W1l-IZ for openwrt-devel@lists.openwrt.org; Tue, 16 Aug 2022 11:24:09 +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 8248E964536 for ; Tue, 16 Aug 2022 13:23:59 +0200 (CEST) From: Jan Venekamp To: openwrt-devel@lists.openwrt.org Subject: [PATCH 3/9] uci: optimize update option in uci_set Date: Tue, 16 Aug 2022 13:23:52 +0200 Message-Id: <20220816112358.75801-4-jan@venekamp.net> X-Mailer: git-send-email 2.32.0 (Apple Git-132) In-Reply-To: <20220816112358.75801-1-jan@venekamp.net> References: <20220816112358.75801-1-jan@venekamp.net> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220816_042406_870793_7AA03852 X-CRM114-Status: GOOD ( 11.03 ) 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 Tue Aug 16 11:23:53 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Venekamp X-Patchwork-Id: 1666801 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=tko2dw8i; 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 4M6TPV3V41z1ygF for ; Tue, 16 Aug 2022 21:26:38 +1000 (AEST) 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=tko2dw8iG2Q3sY XrMUG8ogBORJrxG3cvosZOvYxMXQS8UR9tdAuZ+VX1VWOpwAqXF6taGNv5KwvpAz6Nks2qIbCO9dQ CI9bcHyjn/G8HYWLVB2nCkgSDM7FN8AQaOTW1OWjMS4K4KwHynxBObal+cRY6DkChqVmxgArwI7nq A/cI5asxW0lrUcQFDVDKnbJnTfz5UvCcMxUNOj6VTzsyB/eA2zH4wYDUfCuYmkx1SB2ED0VjeHwUY +/O91y+XoOpMcF/Nozq00IbCoEM3hK+DoOehpQtuciBzth7HqK0r8QpYnlfOvbUVeuj4AzYJuKPCB p/4PzorGqYw2yIV9/WCA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1oNugq-001WW3-8Q; Tue, 16 Aug 2022 11:24:56 +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 1oNug2-001W1k-Pe for openwrt-devel@lists.openwrt.org; Tue, 16 Aug 2022 11:24:09 +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 AC68E96453F for ; Tue, 16 Aug 2022 13:23:59 +0200 (CEST) From: Jan Venekamp To: openwrt-devel@lists.openwrt.org Subject: [PATCH 4/9] uci: fix use-after-free uci_add_list Date: Tue, 16 Aug 2022 13:23:53 +0200 Message-Id: <20220816112358.75801-5-jan@venekamp.net> X-Mailer: git-send-email 2.32.0 (Apple Git-132) In-Reply-To: <20220816112358.75801-1-jan@venekamp.net> References: <20220816112358.75801-1-jan@venekamp.net> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220816_042407_001805_6305E364 X-CRM114-Status: UNSURE ( 8.37 ) 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 Tue Aug 16 11:23:54 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Venekamp X-Patchwork-Id: 1666804 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=S4+znJqs; 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 4M6TQg5GH8z1ygF for ; Tue, 16 Aug 2022 21:27:39 +1000 (AEST) 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=S4+znJqssnS/el yVxrMfaYHM2HCVkK97WqS15Jg9pmuTMBCp1tJe0ld7pwHy2P8mCyUFiqvbPa3/pDCEa+AV6oxVDHN LJrLIFsjlnFgWK8bfkfHqBWREYXxEmF9SHAH8dHk4t6/3gh9VUTU5v71FnLlQMkMGEhKAwcw5E4Bl pMIyuwJgRiTxoLOX9rXll6VCfh845Xk9W0Wu62OamW2J6ps8N3y6kCYnfMMJhsfzxRGE3NDpX6mvi 4Aq3oJa0vDxpLS6flLV9p9NOIbUTsHIhOpwNaleroWeV7Mx7/8W3xrZmesT6ZFLU0xmbGEgdonNqY 6TGAdu9c63CoDJb1IZAg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1oNuhl-001X4i-9r; Tue, 16 Aug 2022 11:25: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 1oNug5-001W6x-KN for openwrt-devel@lists.openwrt.org; Tue, 16 Aug 2022 11:24:11 +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 C07CD964542 for ; Tue, 16 Aug 2022 13:23:59 +0200 (CEST) From: Jan Venekamp To: openwrt-devel@lists.openwrt.org Subject: [PATCH 5/9] uci: fix atomicity of uci_add_list Date: Tue, 16 Aug 2022 13:23:54 +0200 Message-Id: <20220816112358.75801-6-jan@venekamp.net> X-Mailer: git-send-email 2.32.0 (Apple Git-132) In-Reply-To: <20220816112358.75801-1-jan@venekamp.net> References: <20220816112358.75801-1-jan@venekamp.net> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220816_042410_015916_57C38AEB X-CRM114-Status: GOOD ( 13.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: 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 Tue Aug 16 11:23:55 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Venekamp X-Patchwork-Id: 1666803 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=ASAF9MHK; 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 4M6TQ33SWqz1ygF for ; Tue, 16 Aug 2022 21:27:07 +1000 (AEST) 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=ASAF9MHK9fRHi4 miY8dNGBI7u/nfP2AfSrd5Wi0PesUnOUkqdbqvDJly7XTl99xqhyVkPH4AuS7P4gLswYGSfe+ampF +04XmoBF2VwWvLwMq3hy0sqY2ZhhMpM/rxMLjmyvfN6DgNcqJQXjExNas9U5Jlb7/Wn7sIyQuXxcS BAJkH2GCBoFQK0YKWIGBPXo98IwPN6Z41EiTrhBADjvEz4lpywjltAxSiEb3V7nTkXvV0BAHy/bvG swokreKmGtyWvN7p3nQd0Wyp7U81eFi1SI/iuQy+WGNzpwSivXESfVMaU3i98LozYk2nBJMME+3QI nmnKeMCzvXUssclSSzUA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1oNuhN-001Wri-N9; Tue, 16 Aug 2022 11:25: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 1oNug5-001W6y-KV for openwrt-devel@lists.openwrt.org; Tue, 16 Aug 2022 11:24:11 +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 D611B9632FE for ; Tue, 16 Aug 2022 13:23:59 +0200 (CEST) From: Jan Venekamp To: openwrt-devel@lists.openwrt.org Subject: [PATCH 6/9] uci: maintain option position in uci_add_list Date: Tue, 16 Aug 2022 13:23:55 +0200 Message-Id: <20220816112358.75801-7-jan@venekamp.net> X-Mailer: git-send-email 2.32.0 (Apple Git-132) In-Reply-To: <20220816112358.75801-1-jan@venekamp.net> References: <20220816112358.75801-1-jan@venekamp.net> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220816_042409_853845_091CBE22 X-CRM114-Status: UNSURE ( 9.79 ) 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 Tue Aug 16 11:23:56 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Venekamp X-Patchwork-Id: 1666805 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=lhOtzOB8; 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 4M6TRZ4hzlz1ygF for ; Tue, 16 Aug 2022 21:28:26 +1000 (AEST) 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=H7smgNbuq/TI1mujoB1lwVISY30eiWCPPiRAiqS+IRc=; b=lhOtzOB8BQEEek MpY2ZElt6z6SikFmTVeWr4Aj+3jmhX7mrUCg9RQobV9QICAfxZrd5X568Z/Acvh+TS/Bk0GjOaaiM T4oOXn5nlM62c5JeQmB0IhLtYac2ZKIUReZO746UCr+tLsVBwbExYxDJbej8gWzucwFtaSbz+s07I ijhVxg0FlhFJ62e6RZiUSA92WNZ2BGXK61XabhtugMm/lpdc4T1Ep91SGETDU23wrKnJ2WwSylLey h2dx+abovA4hvJB7ADK5xT9zkiAz6GzGRiEiYIKro0pKWytQJSFG7QEpx8ucgOpPEg9+vx4GEE2Vg EsOXpvhyTL3jWXmsythQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1oNuiN-001XUS-MP; Tue, 16 Aug 2022 11:26:32 +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 1oNug5-001W70-MC for openwrt-devel@lists.openwrt.org; Tue, 16 Aug 2022 11:24:12 +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 E90B3964544 for ; Tue, 16 Aug 2022 13:23:59 +0200 (CEST) From: Jan Venekamp To: openwrt-devel@lists.openwrt.org Subject: [PATCH 7/9] uci: fix memory leak uci_set on update section Date: Tue, 16 Aug 2022 13:23:56 +0200 Message-Id: <20220816112358.75801-8-jan@venekamp.net> X-Mailer: git-send-email 2.32.0 (Apple Git-132) In-Reply-To: <20220816112358.75801-1-jan@venekamp.net> References: <20220816112358.75801-1-jan@venekamp.net> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220816_042410_078370_43F54FB1 X-CRM114-Status: GOOD ( 19.04 ) 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..783836b 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, ptr->section, &old->e.list); + uci_section_transfer_options(ptr->s, old); + if (ptr->section == old->e.name) + ptr->section = ptr->o->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 Tue Aug 16 11:23:57 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Venekamp X-Patchwork-Id: 1666806 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=fpMKtjk2; 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 4M6TSD04cVz1ygF for ; Tue, 16 Aug 2022 21:29:00 +1000 (AEST) 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=8jQCDs9VwqiZWtz6zvfVRLew8d8/UkIbxmawFXdwrtM=; b=fpMKtjk28lA5u3 xb1BoZtHgwKWkGT06HeR3WjiSmDWhqVR7mRzjIKq7k76N4WzQLAhfW+DebKccRsqXEr3QdWNot8Ad NWBOBU2i7QQB/R2WdXO0eHYobV4NRohSzNQvNzDOrytCQtHr1uJt6s4hhghQ5Y9JeaReOSMg0wwzj jKt95VaU0AokWcmtQNSpzFBd2PdN0K5RI1wm1S6Uh/jp/Aq0KMwiqUOMnsAv+EXD4YleoYvLJ/eUk Ix0vUmctTLtsdrlBdF4u/F7MIVD1iMQyPb6Tlnz19Hyasnn9XlqHq4mDVhXU73O/elnSyj0eaVsSC pfhWNg5OLclZp6z6I/WQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1oNuiy-001Xus-69; Tue, 16 Aug 2022 11:27:08 +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 1oNug5-001W71-MH for openwrt-devel@lists.openwrt.org; Tue, 16 Aug 2022 11:24:11 +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 06AF9964545 for ; Tue, 16 Aug 2022 13:24:00 +0200 (CEST) From: Jan Venekamp To: openwrt-devel@lists.openwrt.org Subject: [PATCH 8/9] uci: optimize update section in uci_set Date: Tue, 16 Aug 2022 13:23:57 +0200 Message-Id: <20220816112358.75801-9-jan@venekamp.net> X-Mailer: git-send-email 2.32.0 (Apple Git-132) In-Reply-To: <20220816112358.75801-1-jan@venekamp.net> References: <20220816112358.75801-1-jan@venekamp.net> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220816_042409_958129_A6591F35 X-CRM114-Status: GOOD ( 11.29 ) 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 783836b..89c70f0 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, ptr->section, &old->e.list); - uci_section_transfer_options(ptr->s, old); - if (ptr->section == old->e.name) - ptr->section = ptr->o->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, ptr->section, &old->e.list); + uci_section_transfer_options(ptr->s, old); + if (ptr->section == old->e.name) + ptr->section = ptr->o->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 Tue Aug 16 11:23:58 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Venekamp X-Patchwork-Id: 1666808 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=c/GRRAea; 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 4M6TTM4llmz1ygF for ; Tue, 16 Aug 2022 21:29:59 +1000 (AEST) 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=c/GRRAear1IYmE /sFXS5+wKiqvPEXWX6drnrFUpqjIvnuBzrXIVH6iKqS5UYh23Q1tFiKpAy0SM3nu2kaKKcWYOUUAQ s34OyZ4D4+NLlh31HCv5xbIKMaHYQl/ItxuC2NrkgzWk16A+/arP1Xfa/0xwqET8WrGB2fYXQblqb 3yHNGDFlzV1VW9FhRCKn+LGHtxUVF8xIdJ4tAROOakkn4R0tZfHwvE3VKXRZZiVFu7rv9OZu7JxXI Csqr2bAjUwUCnq7ANe6v1k5qxHpzZyA2smq3cQlmz1bPeQo3abvxG6G+w2KYt1zRhfQyD61LXTuni IUOhqysbVi9O3aYzICZg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1oNujr-001YVs-R8; Tue, 16 Aug 2022 11:28:04 +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 1oNug6-001W7r-SK for openwrt-devel@lists.openwrt.org; Tue, 16 Aug 2022 11:24:12 +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 19F5D964543 for ; Tue, 16 Aug 2022 13:24:00 +0200 (CEST) From: Jan Venekamp To: openwrt-devel@lists.openwrt.org Subject: [PATCH 9/9] uci: macro uci_alloc_element not in uci.h Date: Tue, 16 Aug 2022 13:23:58 +0200 Message-Id: <20220816112358.75801-10-jan@venekamp.net> X-Mailer: git-send-email 2.32.0 (Apple Git-132) In-Reply-To: <20220816112358.75801-1-jan@venekamp.net> References: <20220816112358.75801-1-jan@venekamp.net> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220816_042411_077276_DFB0C6E3 X-CRM114-Status: UNSURE ( 9.26 ) 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;