From patchwork Wed Jan 22 05:35:47 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Karthik Nayak X-Patchwork-Id: 13946844 Received: from mail-pl1-f178.google.com (mail-pl1-f178.google.com [209.85.214.178]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 00EB4158553 for ; Wed, 22 Jan 2025 05:36:03 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.178 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737524165; cv=none; b=IIg6WjAZVSSXJBBfL2/tkDqiTKNUxlZp/nMOcywMkbNhD106BTRkTzRUaZJmL2TPNwNru227zjfBOipGrpQZinMQVfebCUkRXIBS0aDzxD4leKcZ0ch/zhFcpS8jBVfj2fKNmvXGRB5Zk7iEfZApge3qBy/YhQ68g9gfS2fV0ZQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737524165; c=relaxed/simple; bh=zHgYt5EMfHPid5ATZrHYDmyotN8vPAhsL4D345D80JE=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=GyJaic7YqpnH3Ptpf8tSJkkppDv3kSX+/d+NF77EwqG47z2OowgB5VK0VBv6CV6gLtHufqkLWM/ACwyRkEGJbyA6pxStINj891lth98ztUif6GpfqUb3AXqoBvLGzb3HdI1mcF+X4AVYhyTp6C5c75y9tRelM/RadQb3bUx5jek= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=cqIA8j3h; arc=none smtp.client-ip=209.85.214.178 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="cqIA8j3h" Received: by mail-pl1-f178.google.com with SMTP id d9443c01a7336-2166360285dso124525315ad.1 for ; Tue, 21 Jan 2025 21:36:03 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1737524163; x=1738128963; darn=vger.kernel.org; h=cc:to:in-reply-to:references:message-id:content-transfer-encoding :mime-version:subject:date:from:from:to:cc:subject:date:message-id :reply-to; bh=28jZfOcxKfK2UzG27PNx/kXejqCrp9DUSkgWXfLGGg0=; b=cqIA8j3h19OgwyA8XVcrOkIB7zBHegj1V3gTpcMcrTkaAR7peVMgg0AqdlM+zYaxJ8 uO3MXJMM0FcKdlJpTyEeNdDMO1u0eZGfMJgVac8G9VZOGbbV5RMiUdZkFS+L1N+MuvzA FM0GME99+CiDJEKdUxv0MbSfPEaTeRTZannOSzui2NuPuBAnZZ1SMPHd0Y3qP2cq4uK5 0jdvskXrLuPJhmCRo4sCTs5Vt0h2wlWDVqE26ZTbLU6bK46CHr70nbde62fKfaJjK2zr anSLO5f4Ffz8cSaZy0EPG6iHssyWJ1CaFs2sC3P3yJ52uPm/43Co9ITe92C9sjK3EesD QHYw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1737524163; x=1738128963; h=cc:to:in-reply-to:references:message-id:content-transfer-encoding :mime-version:subject:date:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=28jZfOcxKfK2UzG27PNx/kXejqCrp9DUSkgWXfLGGg0=; b=YIVXn4EoroGbGRpgG8+SnJjfqdkKjvF2eIx4zZ8ccBa48tTSlld+GTSEc8cFsjT/MB mQyoKjxm1EtJO32GfnCcX0bLgE46HVHwkWOS/bt+rnRSLSEw9RKCAzYUl1bWs+e5zbbm FGX8GajV6XLQDryGhlqqrsYTZBw/AH8Fc6Xfdyy1Z90aP6t++jfufb1lUbfOJ13+601C M1WB+hnQiZBOmfCtxj0mv9jhS5hpK4FxlfBgjqiM2ZTYdKCRbDaoyzy+1AYbbOz4Tm69 u/cgFXRnPjWgu5UGSIdDL1fl6IqtYpt5SDZC2u7hQKCNXkxrfBKXXYEpBGQ6hFX1DZPr T+Dw== X-Gm-Message-State: AOJu0YxnQZ9HaoUOSjuNpe/iEUtUPSicqyi8zs/jcPKwKjSpv5U0SO1N zp7ZFDbGyHEn/Wdav1++i28OD3EnikQ2sXRvCXq5T9O+3tqR1drI X-Gm-Gg: ASbGncvxlNFUEL5/uz7zmzqdhz31182YNtUASjqBeoqhq8SfrD/q9CkSxRE9eSZWpVL uvLtDE3ARwx3J3KkDM+cnXKqCcLnKq1pY9t03dplGpFG8V5Fnj82qtA/PJQ307eshNJlcC4CBTb 5Df0dgmmRo3ov3tTHHUOAG8n3uTdZUOxLY+vur/75Oieu7MTMRrWabdLfDmF/OKqz9T3R6lVdcN aopOrC7UauvyB590vw8/ZDfPLsWlRVXDjn6MuP8hnqbaIJvJg1a+lmxhyiNlqB6fDvp31OO X-Google-Smtp-Source: AGHT+IEV25udoGZ8jIvVnTJCvFHWk12QiyE+mBVHcThrNE8lGdDpmZyIvKg6bxZggH2L59G/6SUP3g== X-Received: by 2002:a05:6a20:12d5:b0:1d9:c615:d1e6 with SMTP id adf61e73a8af0-1eb21177f19mr29884182637.0.1737524163081; Tue, 21 Jan 2025 21:36:03 -0800 (PST) Received: from [127.0.0.2] ([103.39.127.160]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-72db4e718efsm9369207b3a.152.2025.01.21.21.36.01 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 21 Jan 2025 21:36:02 -0800 (PST) From: Karthik Nayak Date: Wed, 22 Jan 2025 06:35:47 +0100 Subject: [PATCH v3 1/3] refs: mark `ref_transaction_update_reflog()` as static Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Message-Id: <20250122-461-corrupted-reftable-followup-v3-1-ae5f88bf04fa@gmail.com> References: <20250122-461-corrupted-reftable-followup-v3-0-ae5f88bf04fa@gmail.com> In-Reply-To: <20250122-461-corrupted-reftable-followup-v3-0-ae5f88bf04fa@gmail.com> To: git@vger.kernel.org Cc: Karthik Nayak , Patrick Steinhardt , Junio C Hamano X-Mailer: b4 0.14.2 X-Developer-Signature: v=1; a=openpgp-sha256; l=2665; i=karthik.188@gmail.com; h=from:subject:message-id; bh=zHgYt5EMfHPid5ATZrHYDmyotN8vPAhsL4D345D80JE=; b=owJ4nAHtARL+kA0DAAoBPtWfJI5GjH8ByyZiAGeQg72OO3PWxNsss5YjgK2QGHsq4uqiAgyDZ LDwbTKb4zSfBokBswQAAQoAHRYhBFfOTH9jdXEPy2XGBj7VnySORox/BQJnkIO9AAoJED7VnySO Rox/5KcMAI4guM4JRs4Kp08GDTwIRCpxYhdB268F513Vu3PcXxxjN0lxfrnmKFcWqHKbg7+GArl KG8BWte2Vf7Kp6JivkKNbyNy0YuVolPmg5qA8TNh53wEuXmLDK+vKXtbKYrcpsGKPwcRkEZFr/n LWaRyEPvzKQkc5QugpuaVloiqQnygRVdRvN1pBZGkiPU+/bNjmYAjT5iqrjxf8AFLbyzDo8maTE s93ksskVb3sw9cjEqa8qTrwHoAIzYKat9rumEon4iznx48xSKjNL4RfsBTPWCBGkjOsc/Iw8JA/ vTKHdAxlW4Pse3HtkuekxRIhfIpCLqK+NEJ57zuGOV6lJKHpMX5QEGgTksi8CUhUG2wYcnsWhVV CrRWMyE0dbMumG0KDzg/aolKVmd0Orp0MU1r3Pj9El1p0zr4W4ntsrCnnqVeUjFsXwV39tP3YoI luvRq/Ynwq5T3T2L36EPWPoospCJyMz5wdhubDjfvRCl/A+NL5KraEdtCCN9kWbKfWBa75VLgu6 dM= X-Developer-Key: i=karthik.188@gmail.com; a=openpgp; fpr=57CE4C7F6375710FCB65C6063ED59F248E468C7F The `ref_transaction_update_reflog()` function is only used within 'refs.c', so mark it as static. Reported-by: Junio C Hamano Signed-off-by: Karthik Nayak --- refs.c | 22 +++++++++++++++------- refs.h | 14 -------------- 2 files changed, 15 insertions(+), 21 deletions(-) diff --git a/refs.c b/refs.c index f7b6f0f897eb58665e10a2efd3eb53c3f72abe61..ad6d774717150f1fe68a59c629e05e49a469693f 100644 --- a/refs.c +++ b/refs.c @@ -1318,13 +1318,21 @@ int ref_transaction_update(struct ref_transaction *transaction, return 0; } -int ref_transaction_update_reflog(struct ref_transaction *transaction, - const char *refname, - const struct object_id *new_oid, - const struct object_id *old_oid, - const char *committer_info, unsigned int flags, - const char *msg, unsigned int index, - struct strbuf *err) +/* + * Similar to`ref_transaction_update`, but this function is only for adding + * a reflog update. Supports providing custom committer information. The index + * field can be utiltized to order updates as desired. When not used, the + * updates default to being ordered by refname. + */ +static int ref_transaction_update_reflog(struct ref_transaction *transaction, + const char *refname, + const struct object_id *new_oid, + const struct object_id *old_oid, + const char *committer_info, + unsigned int flags, + const char *msg, + unsigned int index, + struct strbuf *err) { struct ref_update *update; diff --git a/refs.h b/refs.h index a0cdd99250e8286b55808b697b0a94afac5d8319..09be47afbee51e99f4ae49588cd65596ccfcb07e 100644 --- a/refs.h +++ b/refs.h @@ -771,20 +771,6 @@ int ref_transaction_update(struct ref_transaction *transaction, unsigned int flags, const char *msg, struct strbuf *err); -/* - * Similar to`ref_transaction_update`, but this function is only for adding - * a reflog update. Supports providing custom committer information. The index - * field can be utiltized to order updates as desired. When not used, the - * updates default to being ordered by refname. - */ -int ref_transaction_update_reflog(struct ref_transaction *transaction, - const char *refname, - const struct object_id *new_oid, - const struct object_id *old_oid, - const char *committer_info, unsigned int flags, - const char *msg, unsigned int index, - struct strbuf *err); - /* * Add a reference creation to transaction. new_oid is the value that * the reference should have after the update; it must not be From patchwork Wed Jan 22 05:35:48 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Karthik Nayak X-Patchwork-Id: 13946845 Received: from mail-pj1-f49.google.com (mail-pj1-f49.google.com [209.85.216.49]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 5B974190692 for ; Wed, 22 Jan 2025 05:36:06 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.216.49 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737524167; cv=none; b=PbksFkWLQUeQBEdha85sDtQr6N8tRkPnB0hHwz6ZA0r19FXkyfNNuGJ8hybZDRYwrBwr+MVME+aPprizkiPP2bPIO+EXSti3ePut12F4v0rg7wZArhwm2j9ZYfLrjZ7WuMAZO4lPBe81JjRDZ5dpnN57BkQ59YkjzcB7op5VO4g= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737524167; c=relaxed/simple; bh=67d1Ix8aAmNRTgR/Bvv6TXJRyzwplTwSAZM5YGxA9Gg=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=bB82kc5/rqufggO/KTjMEWyq6lPnPEq3fiOOtSe5s34yJIzVq8hg30rLki+dsOCzQWn20K0zsAoDIyHYBZAQENNn81oZE47M+a9U330GAQCIqO4fiy9MapvcfWSOuE+xMfY5bSHJm6WWWkpyXZOeosiQ+EmhZRBDklHNhxGQH7I= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=S/2BQUyB; arc=none smtp.client-ip=209.85.216.49 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="S/2BQUyB" Received: by mail-pj1-f49.google.com with SMTP id 98e67ed59e1d1-2f43d17b0e3so11712546a91.0 for ; Tue, 21 Jan 2025 21:36:06 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1737524165; x=1738128965; darn=vger.kernel.org; h=cc:to:in-reply-to:references:message-id:content-transfer-encoding :mime-version:subject:date:from:from:to:cc:subject:date:message-id :reply-to; bh=BiUhG0rQ6Qig+Jfqu0diARchfeZC/aytcSuPqquNORA=; b=S/2BQUyBxkNiIv1g4okBDwJC/89e1HbZhMCuXUxZqYtEuultA+yoDT8mc5Xsb9Q4an PPxMZqE3FQR7VdD8/Pb4fPe/6pZSiHXwNrgCuEzLYKk3b70wEQ7WAIQvihzOH9QavtYs yMWPKJKwm+IZJ1rOcQyMzy0UPS5yEVWt5jqX0boMcT5gyhTgsf3thRE9plCBRLeYq9Os xSyNBAdk8/nwgwtHf0jqVBsgLasnL7yjLQYzpOAcrXkh13oJ7m1hw0vcRqE3375QVrMP q8s5LsqLLbcH0DKCTPb2Vqu40DknANF39+cp9lsgY22z2Ww+MnS/kXzLQeWJMMNFmvPA d9iQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1737524165; x=1738128965; h=cc:to:in-reply-to:references:message-id:content-transfer-encoding :mime-version:subject:date:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=BiUhG0rQ6Qig+Jfqu0diARchfeZC/aytcSuPqquNORA=; b=cSKM+mLdFnkNCPcoec2qAVtpgk57h7pGQkhxTXldcW3fTamh0VKClQ9iwD8X9X10S1 GQGcWAQDdhCTAJ62h9pHkGmGc9hZOS1VZGaihESh3bw+op7lrrHsEh9T91tw27yc2q6r BeKNR6lAGRNOaN6KGPt8YNfVoBvLzz/6iDehGyihTnppBa+SHqDjrbIlFD1W9cGpTwop KA+28BXrca3Vy9RbQ8Z5TZh6UYp12LxE4Xz8IPMFGT3rrnHindI1Wh0XBiNuU4XszZud u93l/4u9VWN2Vsf6D5Rs5sxlHYhaQCfyB41ZCvgiFGYWAbPeM4IE5KUwiMMdzO0HY4Mu VkVw== X-Gm-Message-State: AOJu0YzksLccC7wkxErCjtFa0Rwacx5pfHkkAFHYafV0wLQfGSlNg5wN QxcDEE/k0VEJbAYaBNSg6+zRy0td/x/T5evHT2ZVnuSxnyLLpv+n X-Gm-Gg: ASbGncsvAl0bdfGRAFABdmGZYTD9G3cHzV28zASwyyGKonrBdQwhOSL5P6lFEV9zt+F xFW8WFoLMsFYx2OJe9nKiSvhWvZnYQccRmzTHPKLy9fLmXJgyDes5bA2aL9FtPYif6YcQl2QrPR Q0N+na2u0H97SnVFm/Gf5YPKhwZZ2NWdgpUCHs8xUhWyHzf/DPA4FoUr7IR0TLWazq1PKkDuYik LDgk4Gh9Q4PBwe8RqRm9Q9hWuj339TE50py6IdDl6zoGBYbTC/BpQFsJPcY9s3aTZJweWFQ X-Google-Smtp-Source: AGHT+IFd/2bxoqVqcevaMpVMN5ioGq4X1VrIhNEmK+NIzUeNMtOM6cWS34gIn3jPgC4Yj92pGs7SaQ== X-Received: by 2002:a05:6a00:a38d:b0:725:cfd0:dffa with SMTP id d2e1a72fcca58-72daf9beb7emr26234905b3a.5.1737524165416; Tue, 21 Jan 2025 21:36:05 -0800 (PST) Received: from [127.0.0.2] ([103.39.127.160]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-72db4e718efsm9369207b3a.152.2025.01.21.21.36.03 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 21 Jan 2025 21:36:05 -0800 (PST) From: Karthik Nayak Date: Wed, 22 Jan 2025 06:35:48 +0100 Subject: [PATCH v3 2/3] refs: use 'uint64_t' for 'ref_update.index' Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Message-Id: <20250122-461-corrupted-reftable-followup-v3-2-ae5f88bf04fa@gmail.com> References: <20250122-461-corrupted-reftable-followup-v3-0-ae5f88bf04fa@gmail.com> In-Reply-To: <20250122-461-corrupted-reftable-followup-v3-0-ae5f88bf04fa@gmail.com> To: git@vger.kernel.org Cc: Karthik Nayak , Patrick Steinhardt , "brian m. carlson" X-Mailer: b4 0.14.2 X-Developer-Signature: v=1; a=openpgp-sha256; l=2559; i=karthik.188@gmail.com; h=from:subject:message-id; bh=67d1Ix8aAmNRTgR/Bvv6TXJRyzwplTwSAZM5YGxA9Gg=; b=owJ4nAHtARL+kA0DAAoBPtWfJI5GjH8ByyZiAGeQg716kFpezf6w6jW89Z85Gc79C41zsQCOi R1vAQGbjBqeXYkBswQAAQoAHRYhBFfOTH9jdXEPy2XGBj7VnySORox/BQJnkIO9AAoJED7VnySO Rox/NOcL/2b65vcTJ4Lhhsh66daOakfmrcxD0hTHOmzdjvl75g+ZgVrl2tB24EPTff/0N+1Zn6V m1uZxn094dceZ5kwoyVD0EP1SyTkoefXFUC5QJh1e+o+gd1bnuNKQhO7UX1JqtStq2xLAUsgBzJ hIaCZcDMYbu532L/hs4vuhjIxUk5owYVw2x2dFc8UoSZ4aqxJ/tqt9kLtvY3GTLXs97HUN0/eOH +Javmuu0uFRwKogwqUNxbxhgR6PnLe5rfEaFZmhszN5M5qi/9FU1zkpJ3l5FNL3RnlMBDXia1ad 2bA/VOrcg+GvdL6SQg5bsIO20mHj9cXWwz9w+AGWsYeDWVEAXSRAl1Dir8Hdk8wt+V1U5snfPij Lee32WWVQmaHgWNXLnw1tin/zXBRL8lNgdy90I76tvzVUSpAydNIMszuyWaX95DcCqUvjFaesMa YdVdGCAd6GYfFrL/NY9wYlD6n0255idIW1KFxRa0maErtzqXzdgI14nscjsvMe/k7JZYfuSqda9 ak= X-Developer-Key: i=karthik.188@gmail.com; a=openpgp; fpr=57CE4C7F6375710FCB65C6063ED59F248E468C7F The 'ref_update.index' variable is used to store an index for a given reference update. This index is used to order the updates in a predetermined order, while the default ordering is alphabetical as per the refname. For large repositories with millions of references, it should be safer to use 'uint64_t'. Let's do that. This also is applied for all other code sections where we store 'index' and pass it around. Reported-by: brian m. carlson Signed-off-by: Karthik Nayak --- refs.c | 4 ++-- refs/refs-internal.h | 4 ++-- refs/reftable-backend.c | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/refs.c b/refs.c index ad6d774717150f1fe68a59c629e05e49a469693f..ef04f403a6a3a34f9156b4cf68c3daa29c9cbad6 100644 --- a/refs.c +++ b/refs.c @@ -1331,7 +1331,7 @@ static int ref_transaction_update_reflog(struct ref_transaction *transaction, const char *committer_info, unsigned int flags, const char *msg, - unsigned int index, + uint64_t index, struct strbuf *err) { struct ref_update *update; @@ -2813,7 +2813,7 @@ static int migrate_one_ref(const char *refname, const char *referent UNUSED, con } struct reflog_migration_data { - unsigned int index; + uint64_t index; const char *refname; struct ref_store *old_refs; struct ref_transaction *transaction; diff --git a/refs/refs-internal.h b/refs/refs-internal.h index aaab711bb96844755dfa600d37efdb25b30a0765..8894b43d1d1a327d404d3923c507d2d39649de19 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -120,7 +120,7 @@ struct ref_update { * when migrating reflogs and we want to ensure we carry over the * same order. */ - unsigned int index; + uint64_t index; /* * If this ref_update was split off of a symref update via @@ -203,7 +203,7 @@ struct ref_transaction { enum ref_transaction_state state; void *backend_data; unsigned int flags; - unsigned int max_index; + uint64_t max_index; }; /* diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index 289496058e6eb4d3e3aef96ca70219cd4ff78eae..6814c87bc618229ac8a70b904be3f850371ad876 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -942,7 +942,7 @@ struct write_transaction_table_arg { size_t updates_nr; size_t updates_alloc; size_t updates_expected; - unsigned int max_index; + uint64_t max_index; }; struct reftable_transaction_data { From patchwork Wed Jan 22 05:35:49 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Karthik Nayak X-Patchwork-Id: 13946846 Received: from mail-pl1-f178.google.com (mail-pl1-f178.google.com [209.85.214.178]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 9002C170826 for ; Wed, 22 Jan 2025 05:36:09 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.178 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737524171; cv=none; b=b+p+/Rd4axGsTtdXoxtcRR139+eKjbQIqV1ue7zBfvJFkYY8NbpiA+v3UdNeFEdclZT/VhiSS71G/ukf180zrJvTzUJYmdFDs9/AIBZxwimpP5GEsj8TxywA++96c7lLOmJS0Hz2SuqU6BFmr8l6NBoBybsPQ+FxEeXXbOVfHh8= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737524171; c=relaxed/simple; bh=zn+jsCfxo1JBK34Hji45uwsLF+WzAwQtQiARZAzzUJw=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=YBbqntqM6UlgwXhKFMsf2B5bWGK0JWqWQM9tvKpCo8OGBCxQcDVXEWGPgCzxnZFrrD/lraD4pGGKBGktz1pCaWeE1F+eyze65QLI7GYlB23lhgqxZFlAKAKaDmo8jQNaRQaWmzYuCOwQEKhDWq4kmnY0hv60bBtbjDbsHyWTYwc= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=NWukDbly; arc=none smtp.client-ip=209.85.214.178 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="NWukDbly" Received: by mail-pl1-f178.google.com with SMTP id d9443c01a7336-2162c0f6a39so9064005ad.0 for ; Tue, 21 Jan 2025 21:36:09 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1737524169; x=1738128969; darn=vger.kernel.org; h=cc:to:in-reply-to:references:message-id:content-transfer-encoding :mime-version:subject:date:from:from:to:cc:subject:date:message-id :reply-to; bh=YnvgPaFKbJZIpZWQHBD/HDrx1fTvfA+buSmX6eF9l5Y=; b=NWukDblyUxXoeob7ByUqUyRIknrEkL4qJIvoAKbrr6f3+V4jokstgsVFfIjUpa39NX 0/LmciRs+2ARsxlVJaSJSf8xsBAsGxgnUzY9Q12gnO6XvYEWERQdVA+nsC4cDSZZlHfY o6k5s+hhP+w/nJexiLGnGIu6fJeAGvOtFE+eJLsqjBBUn/scYGuVVcaY3X/TY01w3nAi GkUVjXec0X2kx5kAy1pU0zp9KZThdIqovAKskhMXTq4Ws7q76HqJk4p2Cm02ZK4r/kj7 hAgRJQyQAwvaqIZgpbrcgKYc7Kni5lsNuO5dohNifiNFDhUMuetclc5JIPsbHGSJVi60 uvpw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1737524169; x=1738128969; h=cc:to:in-reply-to:references:message-id:content-transfer-encoding :mime-version:subject:date:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=YnvgPaFKbJZIpZWQHBD/HDrx1fTvfA+buSmX6eF9l5Y=; b=HHiVrtJgMz3D9C/STWh2G7TcVUyVC4oYdf/HZ8IreEsiQPqsZxXpZRlZCjI9HWyMqQ QXu15PBKwCxVCqusRu4soP71Pa1FTERuYkMqHasm+2MJiSosFMp0JSF3gXoX+vRw7Ybg U6lgeacUMIeRPYawziXoWWRKzuUj6rPdSgG75mYlJUybuQ5/QW8arH0AciwACr3SpqqS McamaY02FjSpRIQvG+hgEomgDggA9kAhtdtnGCa5E/wzE7M2ufMhqn0ZizgZ0iOkQfut kHVLwzBGIeNHRg7rff5aarQiJep6jFKkVChE0oUDQHv/mobKyPqdy1KFgNPgMJSpcDE9 8sWA== X-Gm-Message-State: AOJu0Yz3gEfCqUylj5/Nm0I09CoDLG++zy4C6vYUd10n3zFvpIinxkjp M4JD3FyXvemEYVPAUZtEHBZ9oeNL0H1i/3kw4RxUiawi8G0EL6uTeO9A+Lbl X-Gm-Gg: ASbGncv+sxFZvTjzrpUJyPHAhYgrK3A8DMKZmlg8PMTXrxBD7t5bbV8jvYViEnWaFKN wV2Ogt0L0XUZFcbRy/qkmD9bX6IPgEfgSG2/4VvMK5N/8ErAd/lWeKNRLKLyOy9/U2TcVpmVkCt Oivi+G25ui9xq3PotHuJHx6dFL04YDLwj8RGwCTwDoQje/hO0fWd1R9kIVPSQ5RlXe8X7Y37IzL gsGFd07z/K56eulaAhpkZjBTVkT/wWLC9BJFvznLYvRucgM00rT0hsNb4l8MbstsO50gfyt X-Google-Smtp-Source: AGHT+IHv5aW8PeOMbRsgPsikUslK19W+dRTMdQNdNQ6PfYVpocd1StB1xCuMxa1JUByxgInr1WyJvw== X-Received: by 2002:a05:6a00:66cb:b0:727:39a4:30cc with SMTP id d2e1a72fcca58-72d8c46e973mr38657133b3a.1.1737524168603; Tue, 21 Jan 2025 21:36:08 -0800 (PST) Received: from [127.0.0.2] ([103.39.127.160]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-72db4e718efsm9369207b3a.152.2025.01.21.21.36.05 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 21 Jan 2025 21:36:07 -0800 (PST) From: Karthik Nayak Date: Wed, 22 Jan 2025 06:35:49 +0100 Subject: [PATCH v3 3/3] reftable: prevent 'update_index' changes after adding records Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Message-Id: <20250122-461-corrupted-reftable-followup-v3-3-ae5f88bf04fa@gmail.com> References: <20250122-461-corrupted-reftable-followup-v3-0-ae5f88bf04fa@gmail.com> In-Reply-To: <20250122-461-corrupted-reftable-followup-v3-0-ae5f88bf04fa@gmail.com> To: git@vger.kernel.org Cc: Karthik Nayak , Patrick Steinhardt X-Mailer: b4 0.14.2 X-Developer-Signature: v=1; a=openpgp-sha256; l=11286; i=karthik.188@gmail.com; h=from:subject:message-id; bh=zn+jsCfxo1JBK34Hji45uwsLF+WzAwQtQiARZAzzUJw=; b=owJ4nAHtARL+kA0DAAoBPtWfJI5GjH8ByyZiAGeQg75o1zZ6eWstgI+SceD0yp2geR5pQXZES Zt9/jhqcXSjNokBswQAAQoAHRYhBFfOTH9jdXEPy2XGBj7VnySORox/BQJnkIO+AAoJED7VnySO Rox/PYUL/RGs89lm4lxjsbE7JuyUtA54noGlERFcvlKuAGrGnxdIGuj0jGsz5jmsh3eUpiIszY8 mjqir8EffopGbG1iYVWseAqi+giZOyzXeAw7Fc9ECMV7tDcp6FBkKNs8zJnEwrcYwQ2s1+lnofc Hq0bDrKCtI36bNCGaR6YnycBHhPK+hP/JuzDIUu/hKJaI1Wr1HUxdk9GVkGn4f4/5b0cxFxEsxx l8mp4uLZj4s2L+p+OktHxosJjrsXWmKdiSmyUoexDCeBJeyEOgjl6HctBeZZTv4QlvM45suC97d O2ws9yT3sJlBHEkU8DWPjXL9hms3tyAs9GXRzkoveb2p7i5nvocGpQO1ANiTfXxAQrQkq5itwSo 1r7HyayDzTwrWgZg36PAKq4934sqYVOqzlAa4QTrjz0hLnhRlge9yLv2bxOOx2paqBr3Vm7Xmm2 6Q5YphKhkupwHRIyKdpVYWHfnoCZHNmrcUF7O0vD22pxpKBUw+9ITziqe/pat3VbStHmlXOkpD5 vw= X-Developer-Key: i=karthik.188@gmail.com; a=openpgp; fpr=57CE4C7F6375710FCB65C6063ED59F248E468C7F The function `reftable_writer_set_limits()` allows updating the 'min_update_index' and 'max_update_index' of a reftable writer. These values are written to both the writer's header and footer. Since the header is written during the first block write, any subsequent changes to the update index would create a mismatch between the header and footer values. The footer would contain the newer values while the header retained the original ones. To protect against this bug, prevent callers from updating these values after any record is written. To do this, modify the function to return an error whenever the limits are modified after any record adds. Check for record adds within `reftable_writer_set_limits()` by checking the `last_key` and `next` variable. The former is updated after each record added, but is reset at certain points. The latter is set after writing the first block. Modify all callers of the function to anticipate a return type and handle it accordingly. Add a unit test to also ensure the function returns the error as expected. Helped-by: Patrick Steinhardt Signed-off-by: Karthik Nayak --- refs/reftable-backend.c | 20 +++++++++++---- reftable/reftable-error.h | 1 + reftable/reftable-writer.h | 24 ++++++++++-------- reftable/stack.c | 6 +++-- reftable/writer.c | 15 +++++++++++- t/unit-tests/t-reftable-stack.c | 54 ++++++++++++++++++++++++++++++++++++++--- 6 files changed, 99 insertions(+), 21 deletions(-) diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index 6814c87bc618229ac8a70b904be3f850371ad876..9cfb0cb26721a9425c3b4a374f7b41e192037315 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -1443,7 +1443,9 @@ static int write_transaction_table(struct reftable_writer *writer, void *cb_data * multiple entries. Each entry will contain a different update_index, * so set the limits accordingly. */ - reftable_writer_set_limits(writer, ts, ts + arg->max_index); + ret = reftable_writer_set_limits(writer, ts, ts + arg->max_index); + if (ret < 0) + goto done; for (i = 0; i < arg->updates_nr; i++) { struct reftable_transaction_update *tx_update = &arg->updates[i]; @@ -1766,7 +1768,9 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data) deletion_ts = creation_ts = reftable_stack_next_update_index(arg->be->stack); if (arg->delete_old) creation_ts++; - reftable_writer_set_limits(writer, deletion_ts, creation_ts); + ret = reftable_writer_set_limits(writer, deletion_ts, creation_ts); + if (ret < 0) + goto done; /* * Add the new reference. If this is a rename then we also delete the @@ -2298,7 +2302,9 @@ static int write_reflog_existence_table(struct reftable_writer *writer, if (ret <= 0) goto done; - reftable_writer_set_limits(writer, ts, ts); + ret = reftable_writer_set_limits(writer, ts, ts); + if (ret < 0) + goto done; /* * The existence entry has both old and new object ID set to the @@ -2357,7 +2363,9 @@ static int write_reflog_delete_table(struct reftable_writer *writer, void *cb_da uint64_t ts = reftable_stack_next_update_index(arg->stack); int ret; - reftable_writer_set_limits(writer, ts, ts); + ret = reftable_writer_set_limits(writer, ts, ts); + if (ret < 0) + goto out; ret = reftable_stack_init_log_iterator(arg->stack, &it); if (ret < 0) @@ -2434,7 +2442,9 @@ static int write_reflog_expiry_table(struct reftable_writer *writer, void *cb_da if (arg->records[i].value_type == REFTABLE_LOG_UPDATE) live_records++; - reftable_writer_set_limits(writer, ts, ts); + ret = reftable_writer_set_limits(writer, ts, ts); + if (ret < 0) + return ret; if (!is_null_oid(&arg->update_oid)) { struct reftable_ref_record ref = {0}; diff --git a/reftable/reftable-error.h b/reftable/reftable-error.h index f4048265629fe456207b88620658193f770a84f0..a7e33d964d0cfe5546f588d26c0fcb66ab326828 100644 --- a/reftable/reftable-error.h +++ b/reftable/reftable-error.h @@ -30,6 +30,7 @@ enum reftable_error { /* Misuse of the API: * - on writing a record with NULL refname. + * - on writing a record before setting the writer limits. * - on writing a reftable_ref_record outside the table limits * - on writing a ref or log record before the stack's * next_update_inde*x diff --git a/reftable/reftable-writer.h b/reftable/reftable-writer.h index 5f9afa620bb00de66c311765fb0ae8c6f56401ae..1ea014d389cc47f173279e3234a82f3fcbc807a0 100644 --- a/reftable/reftable-writer.h +++ b/reftable/reftable-writer.h @@ -124,17 +124,21 @@ int reftable_writer_new(struct reftable_writer **out, int (*flush_func)(void *), void *writer_arg, const struct reftable_write_options *opts); -/* Set the range of update indices for the records we will add. When writing a - table into a stack, the min should be at least - reftable_stack_next_update_index(), or REFTABLE_API_ERROR is returned. - - For transactional updates to a stack, typically min==max, and the - update_index can be obtained by inspeciting the stack. When converting an - existing ref database into a single reftable, this would be a range of - update-index timestamps. +/* + * Set the range of update indices for the records we will add. When writing a + * table into a stack, the min should be at least + * reftable_stack_next_update_index(), or REFTABLE_API_ERROR is returned. + * + * For transactional updates to a stack, typically min==max, and the + * update_index can be obtained by inspeciting the stack. When converting an + * existing ref database into a single reftable, this would be a range of + * update-index timestamps. + * + * The function should be called before adding any records to the writer. If not + * it will fail with REFTABLE_API_ERROR. */ -void reftable_writer_set_limits(struct reftable_writer *w, uint64_t min, - uint64_t max); +int reftable_writer_set_limits(struct reftable_writer *w, uint64_t min, + uint64_t max); /* Add a reftable_ref_record. The record should have names that come after diff --git a/reftable/stack.c b/reftable/stack.c index 531660a49f0948c33041831ee0d740feacb22b2f..9649dbbb04c51e106ee752f14481bbad381cb348 100644 --- a/reftable/stack.c +++ b/reftable/stack.c @@ -1058,8 +1058,10 @@ static int stack_write_compact(struct reftable_stack *st, for (size_t i = first; i <= last; i++) st->stats.bytes += st->readers[i]->size; - reftable_writer_set_limits(wr, st->readers[first]->min_update_index, - st->readers[last]->max_update_index); + err = reftable_writer_set_limits(wr, st->readers[first]->min_update_index, + st->readers[last]->max_update_index); + if (err < 0) + goto done; err = reftable_merged_table_new(&mt, st->readers + first, subtabs_len, st->opts.hash_id); diff --git a/reftable/writer.c b/reftable/writer.c index 740c98038eaf883258bef4988f78977ac7e4a75a..76e24018172fc1d80a6535698757979e53b0e213 100644 --- a/reftable/writer.c +++ b/reftable/writer.c @@ -179,11 +179,24 @@ int reftable_writer_new(struct reftable_writer **out, return 0; } -void reftable_writer_set_limits(struct reftable_writer *w, uint64_t min, +int reftable_writer_set_limits(struct reftable_writer *w, uint64_t min, uint64_t max) { + /* + * Set the min/max update index limits for the reftable writer. + * This must be called before adding any records, since: + * - The 'next' field gets set after writing the first block. + * - The 'last_key' field updates with each new record (but resets + * after sections). + * Returns REFTABLE_API_ERROR if called after writing has begun. + */ + if (w->next || w->last_key.len) + return REFTABLE_API_ERROR; + w->min_update_index = min; w->max_update_index = max; + + return 0; } static void writer_release(struct reftable_writer *w) diff --git a/t/unit-tests/t-reftable-stack.c b/t/unit-tests/t-reftable-stack.c index aeec195b2b1014445d71c5db39a9795017fd8ff2..c3f0059c346edbe1ad543c9832959c6fc0aa9180 100644 --- a/t/unit-tests/t-reftable-stack.c +++ b/t/unit-tests/t-reftable-stack.c @@ -103,7 +103,8 @@ static void t_read_file(void) static int write_test_ref(struct reftable_writer *wr, void *arg) { struct reftable_ref_record *ref = arg; - reftable_writer_set_limits(wr, ref->update_index, ref->update_index); + check(!reftable_writer_set_limits(wr, ref->update_index, + ref->update_index)); return reftable_writer_add_ref(wr, ref); } @@ -143,7 +144,8 @@ static int write_test_log(struct reftable_writer *wr, void *arg) { struct write_log_arg *wla = arg; - reftable_writer_set_limits(wr, wla->update_index, wla->update_index); + check(!reftable_writer_set_limits(wr, wla->update_index, + wla->update_index)); return reftable_writer_add_log(wr, wla->log); } @@ -961,7 +963,7 @@ static void t_reflog_expire(void) static int write_nothing(struct reftable_writer *wr, void *arg UNUSED) { - reftable_writer_set_limits(wr, 1, 1); + check(!reftable_writer_set_limits(wr, 1, 1)); return 0; } @@ -1369,11 +1371,57 @@ static void t_reftable_stack_reload_with_missing_table(void) clear_dir(dir); } +static int write_limits_after_ref(struct reftable_writer *wr, void *arg) +{ + struct reftable_ref_record *ref = arg; + check(!reftable_writer_set_limits(wr, ref->update_index, ref->update_index)); + check(!reftable_writer_add_ref(wr, ref)); + return reftable_writer_set_limits(wr, ref->update_index, ref->update_index); +} + +static void t_reftable_invalid_limit_updates(void) +{ + struct reftable_ref_record ref = { + .refname = (char *) "HEAD", + .update_index = 1, + .value_type = REFTABLE_REF_SYMREF, + .value.symref = (char *) "master", + }; + struct reftable_write_options opts = { + .default_permissions = 0660, + }; + struct reftable_addition *add = NULL; + char *dir = get_tmp_dir(__LINE__); + struct reftable_stack *st = NULL; + int err; + + err = reftable_new_stack(&st, dir, &opts); + check(!err); + + reftable_addition_destroy(add); + + err = reftable_stack_new_addition(&add, st, 0); + check(!err); + + /* + * write_limits_after_ref also updates the update indexes after adding + * the record. This should cause an err to be returned, since the limits + * must be set at the start. + */ + err = reftable_addition_add(add, write_limits_after_ref, &ref); + check_int(err, ==, REFTABLE_API_ERROR); + + reftable_addition_destroy(add); + reftable_stack_destroy(st); + clear_dir(dir); +} + int cmd_main(int argc UNUSED, const char *argv[] UNUSED) { TEST(t_empty_add(), "empty addition to stack"); TEST(t_read_file(), "read_lines works"); TEST(t_reflog_expire(), "expire reflog entries"); + TEST(t_reftable_invalid_limit_updates(), "prevent limit updates after adding records"); TEST(t_reftable_stack_add(), "add multiple refs and logs to stack"); TEST(t_reftable_stack_add_one(), "add a single ref record to stack"); TEST(t_reftable_stack_add_performs_auto_compaction(), "addition to stack triggers auto-compaction");