From patchwork Sun Nov 20 01:09: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: 1706687 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=0XSEEWNc; 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 4NFCKx05nYz23lt for ; Sun, 20 Nov 2022 12:17:08 +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=hMyVkhsn6h+mMTtVLL1HWYH/HL9lgxBLYA+b6aIKVeU=; b=0XSEEWNcziauC4 HzCH9Ao4xSSs5JfZmZwHJ/ZBFzWP/AiEGyZrThjYFQ7DoAHMs47IQl3+ciATdmdYGYm8ZtjjT4+Se Gouv1jal90wr1RKnXjaJOYxkTA9n71le+qKvH213uRIHG94RQO52UptKXt6nUn/YxvdnykvQY+V+b NFazG9v0GGGaxS5amAZSTCnUlwQxUN8nztop+qLq+WgIqqVafi3O7RqHX2e7/VwYKUxqK8FRVirLe yVome+CGwTPok6nBZq8Mc5Cd5PZZPfQgnBPdKcr4HhZDFRwA7JFfwD2w275BqhDFm4FkuRm4DVUKK 3gLJOJ0y6cGkG4CsdjXQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1owYul-000r8q-CI; Sun, 20 Nov 2022 01:14:31 +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 1owYpt-000pCm-Dq for openwrt-devel@lists.openwrt.org; Sun, 20 Nov 2022 01:09:31 +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 4FAC7A5ED60 for ; Sun, 20 Nov 2022 02:09:28 +0100 (CET) From: Jan Venekamp To: openwrt-devel@lists.openwrt.org Subject: [PATCH 1/1] uci: ignore wrong section / option name in uci_ptr Date: Sun, 20 Nov 2022 02:09:27 +0100 Message-Id: <20221120010927.23856-2-jan@venekamp.net> X-Mailer: git-send-email 2.32.0 (Apple Git-132) In-Reply-To: <20221120010927.23856-1-jan@venekamp.net> References: <20221120010927.23856-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_170929_773331_D269EA74 X-CRM114-Status: GOOD ( 13.64 ) 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: Several functions that have a uci_ptr as argument expect if ptr->s / ptr->o are provided that strcmp(ptr->s->e.name, ptr->section) == 0 and strcmp(ptr->o->e.name, ptr->option) == 0. Normally this is e [...] 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 Several functions that have a uci_ptr as argument expect if ptr->s / ptr->o are provided that strcmp(ptr->s->e.name, ptr->section) == 0 and strcmp(ptr->o->e.name, ptr->option) == 0. Normally this is ensured by a call to uci_lookup_ptr that precedes the function invocation and by uci_expand_ptr. However, when using the C api directly ptr->section and ptr->option are not guarantied to be correct which can lead to unexpected or inconsistent behaviour (breaking delta tracking). Fix this by using ptr->section and ptr->option exclusively on new section, new option and in the case of uci_revert. In other cases use ptr->s->e.name and ptr->o->e.name exclusively. Signed-off-by: Jan Venekamp --- list.c | 61 ++++++++++++++++++++++++---------------------------------- 1 file changed, 25 insertions(+), 36 deletions(-) diff --git a/list.c b/list.c index 1640213..87b02ad 100644 --- a/list.c +++ b/list.c @@ -267,22 +267,6 @@ uci_free_package(struct uci_package **package) *package = NULL; } -static void -uci_free_any(struct uci_element **e) -{ - switch((*e)->type) { - case UCI_TYPE_SECTION: - uci_free_section(uci_to_section(*e)); - break; - case UCI_TYPE_OPTION: - uci_free_option(uci_to_option(*e)); - break; - default: - break; - } - *e = NULL; -} - __private struct uci_element * uci_lookup_list(struct uci_list *list, const char *name) { @@ -500,7 +484,7 @@ int uci_rename(struct uci_context *ctx, struct uci_ptr *ptr) UCI_ASSERT(ctx, ptr->value); if (!internal && p->has_delta) - uci_add_delta(ctx, &p->delta, UCI_CMD_RENAME, ptr->section, ptr->option, ptr->value); + uci_add_delta(ctx, &p->delta, UCI_CMD_RENAME, ptr->s->e.name, ptr->o ? ptr->o->e.name : NULL, ptr->value); n = uci_strdup(ctx, ptr->value); free(e->name); @@ -552,25 +536,25 @@ int uci_delete(struct uci_context *ctx, struct uci_ptr *ptr) /* NB: pass on internal flag to uci_del_element */ bool internal = ctx && ctx->internal; struct uci_package *p; - struct uci_element *e1, *e2, *tmp; + struct uci_element *e, *tmp; int index; UCI_HANDLE_ERR(ctx); - e1 = uci_expand_ptr(ctx, ptr, true); + uci_expand_ptr(ctx, ptr, true); p = ptr->p; UCI_ASSERT(ctx, ptr->s); - if (ptr->o && ptr->o->type == UCI_TYPE_LIST && ptr->value && *ptr->value) { + if (ptr->o && ptr->o->type == UCI_TYPE_LIST && ptr->value && ptr->value[0]) { if (!sscanf(ptr->value, "%d", &index)) return 1; - uci_foreach_element_safe(&ptr->o->v.list, tmp, e2) { + uci_foreach_element_safe(&ptr->o->v.list, tmp, e) { if (index == 0) { if (!internal && p->has_delta) - uci_add_delta(ctx, &p->delta, UCI_CMD_REMOVE, ptr->section, ptr->option, ptr->value); - uci_free_option(uci_to_option(e2)); + uci_add_delta(ctx, &p->delta, UCI_CMD_REMOVE, ptr->s->e.name, ptr->o->e.name, ptr->value); + uci_free_option(uci_to_option(e)); return 0; } index--; @@ -580,14 +564,19 @@ int uci_delete(struct uci_context *ctx, struct uci_ptr *ptr) } if (!internal && p->has_delta) - uci_add_delta(ctx, &p->delta, UCI_CMD_REMOVE, ptr->section, ptr->option, NULL); + uci_add_delta(ctx, &p->delta, UCI_CMD_REMOVE, ptr->s->e.name, ptr->o ? ptr->o->e.name : NULL, NULL); - uci_free_any(&e1); - - if (ptr->option) + if (ptr->o) { + if (ptr->option == ptr->o->e.name) + ptr->option = NULL; + uci_free_option(ptr->o); ptr->o = NULL; - else if (ptr->section) + } else { + if (ptr->section == ptr->s->e.name) + ptr->section = NULL; + uci_free_section(ptr->s); ptr->s = NULL; + } return 0; } @@ -622,7 +611,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, &old->e.list); + ptr->o = uci_alloc_list(ptr->s, old->e.name, &old->e.list); UCI_TRAP_RESTORE(ctx); uci_list_add(&ptr->o->v.list, &e2->list); @@ -637,7 +626,7 @@ int uci_add_list(struct uci_context *ctx, struct uci_ptr *ptr) 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); + uci_add_delta(ctx, &ptr->p->delta, UCI_CMD_LIST_ADD, ptr->s->e.name, ptr->o->e.name, ptr->value); return 0; error: @@ -661,7 +650,7 @@ int uci_del_list(struct uci_context *ctx, struct uci_ptr *ptr) UCI_ASSERT(ctx, ptr->s); UCI_ASSERT(ctx, ptr->value); - if (!(ptr->o && ptr->option)) + if (!(ptr->o && ptr->o->e.name)) return 0; if ((ptr->o->type != UCI_TYPE_LIST)) @@ -669,7 +658,7 @@ int uci_del_list(struct uci_context *ctx, struct uci_ptr *ptr) p = ptr->p; if (!internal && p->has_delta) - uci_add_delta(ctx, &p->delta, UCI_CMD_LIST_DEL, ptr->section, ptr->option, ptr->value); + uci_add_delta(ctx, &p->delta, UCI_CMD_LIST_DEL, ptr->s->e.name, ptr->o->e.name, ptr->value); uci_foreach_element_safe(&ptr->o->v.list, tmp, e) { if (!strcmp(ptr->value, uci_to_option(e)->e.name)) { @@ -712,7 +701,7 @@ int uci_set(struct uci_context *ctx, struct uci_ptr *ptr) } else if (!ptr->s && ptr->section) { /* new 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 */ + } else if (ptr->o && ptr->o->e.name) { /* update option */ if (ptr->o->type == UCI_TYPE_STRING && !strcmp(ptr->o->v.string, ptr->value)) return 0; @@ -720,13 +709,13 @@ int uci_set(struct uci_context *ctx, struct uci_ptr *ptr) 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); + ptr->o = uci_alloc_option(ptr->s, old->e.name, 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 */ + } else if (ptr->s && ptr->s->e.name) { /* update section */ if (!strcmp(ptr->s->type, ptr->value)) return 0; @@ -747,7 +736,7 @@ int uci_set(struct uci_context *ctx, struct uci_ptr *ptr) } if (!internal && ptr->p->has_delta) - uci_add_delta(ctx, &ptr->p->delta, UCI_CMD_CHANGE, ptr->section, ptr->option, ptr->value); + uci_add_delta(ctx, &ptr->p->delta, UCI_CMD_CHANGE, ptr->s->e.name, ptr->o ? ptr->o->e.name : NULL, ptr->value); return 0; }