From patchwork Sat Mar 2 19:03:46 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Junio C Hamano X-Patchwork-Id: 13579588 Received: from pb-smtp21.pobox.com (pb-smtp21.pobox.com [173.228.157.53]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 698C726AD8 for ; Sat, 2 Mar 2024 19:03:57 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=173.228.157.53 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1709406238; cv=none; b=YOPvIhrgo0SRtnvKqTbfDSmC59IFhC66vFKR4Q9/776FcN5T66tV0fYNMAFj/hvuoB6Jk5OvdkPQWZJNZYn8rcyIUPrmZjiy/+/faNxHjxs2/zYwxYfNkV5NXwlj9x5Sde5k0SRHaTWV20zqqdkTOF669xQ336NxpJy3rb0TlYU= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1709406238; c=relaxed/simple; bh=bfBLdkhvrjwwqfFtC8El3Pa5YQgL7Kuu8t+g5D8ajXI=; h=From:To:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=X+JVzwvIaGCHM+REwXVLRyorQDRU91BArn/FVyncn4uTbVZ8jKu2N0uZIIIIkY7SVGna9zzA7FOSq7ZouuyJpdcy9mte3B3P+h0Tps/lwTFl7l6dY9qE4FGdu2vDLc3Q1DnuvfGPgDJOsGCksjWCRnchXcZvoqFoNePkkGU5mJs= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=pobox.com; spf=pass smtp.mailfrom=pobox.com; dkim=pass (1024-bit key) header.d=pobox.com header.i=@pobox.com header.b=J8iIDt9G; arc=none smtp.client-ip=173.228.157.53 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=pobox.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pobox.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=pobox.com header.i=@pobox.com header.b="J8iIDt9G" Received: from pb-smtp21.pobox.com (unknown [127.0.0.1]) by pb-smtp21.pobox.com (Postfix) with ESMTP id D5B8126D0C; Sat, 2 Mar 2024 14:03:56 -0500 (EST) (envelope-from gitster@pobox.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed; d=pobox.com; h=from:to :subject:date:message-id:in-reply-to:references:mime-version :content-transfer-encoding; s=sasl; bh=bfBLdkhvrjwwqfFtC8El3Pa5Y QgL7Kuu8t+g5D8ajXI=; b=J8iIDt9GohV1ixXpgduAPZBnb1aH59vSuFkvcC4f5 e7XBdhYq0U+KGasm1+9toW4US86667uFY+zsCZhCw4v/xWBmJBIGMf1FfiJ4U4/I Ovhgj30Cvo1Tdvcijs4xM2PZneut2E3siiNcC8rTO8cMZVA+TK1/nXn4NxqARat+ M8= Received: from pb-smtp21.sea.icgroup.com (unknown [127.0.0.1]) by pb-smtp21.pobox.com (Postfix) with ESMTP id CDE6F26D0B; Sat, 2 Mar 2024 14:03:56 -0500 (EST) (envelope-from gitster@pobox.com) Received: from pobox.com (unknown [34.125.176.30]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by pb-smtp21.pobox.com (Postfix) with ESMTPSA id 6A73C26D09; Sat, 2 Mar 2024 14:03:53 -0500 (EST) (envelope-from gitster@pobox.com) From: Junio C Hamano To: git@vger.kernel.org Subject: [PATCH 1/3] unpack: replace xwrite() loop with write_in_full() Date: Sat, 2 Mar 2024 11:03:46 -0800 Message-ID: <20240302190348.3946569-2-gitster@pobox.com> X-Mailer: git-send-email 2.44.0-84-gb387623c12 In-Reply-To: <20240302190348.3946569-1-gitster@pobox.com> References: <20240302190348.3946569-1-gitster@pobox.com> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Pobox-Relay-ID: 9CB8A4CA-D8C7-11EE-9390-A19503B9AAD1-77302942!pb-smtp21.pobox.com We have two packfile stream consumers, index-pack and unpack-objects, that allow excess payload after the packfile stream data. Their code to relay excess data hasn't changed significantly since their original implementation that appeared in 67e5a5ec (git-unpack-objects: re-write to read from stdin, 2005-06-28) and 9bee2478 (mimic unpack-objects when --stdin is used with index-pack, 2006-10-25). These code blocks contain hand-rolled loops using xwrite(), written before our write_in_full() helper existed. This helper now provides the same functionality. Replace these loops with write_in_full() for shorter, clearer code. Update related variables accordingly. Signed-off-by: Junio C Hamano --- builtin/index-pack.c | 17 +++-------------- builtin/unpack-objects.c | 8 +------- 2 files changed, 4 insertions(+), 21 deletions(-) diff --git a/builtin/index-pack.c b/builtin/index-pack.c index a3a37bd215..856428fef9 100644 --- a/builtin/index-pack.c +++ b/builtin/index-pack.c @@ -1524,14 +1524,12 @@ static void final(const char *final_pack_name, const char *curr_pack_name, struct strbuf pack_name = STRBUF_INIT; struct strbuf index_name = STRBUF_INIT; struct strbuf rev_index_name = STRBUF_INIT; - int err; if (!from_stdin) { close(input_fd); } else { fsync_component_or_die(FSYNC_COMPONENT_PACK, output_fd, curr_pack_name); - err = close(output_fd); - if (err) + if (close(output_fd)) die_errno(_("error while closing pack file")); } @@ -1566,17 +1564,8 @@ static void final(const char *final_pack_name, const char *curr_pack_name, write_or_die(1, buf.buf, buf.len); strbuf_release(&buf); - /* - * Let's just mimic git-unpack-objects here and write - * the last part of the input buffer to stdout. - */ - while (input_len) { - err = xwrite(1, input_buffer + input_offset, input_len); - if (err <= 0) - break; - input_len -= err; - input_offset += err; - } + /* Write the last part of the buffer to stdout */ + write_in_full(1, input_buffer + input_offset, input_len); } strbuf_release(&rev_index_name); diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c index e0a701f2b3..f1c85a00ae 100644 --- a/builtin/unpack-objects.c +++ b/builtin/unpack-objects.c @@ -679,13 +679,7 @@ int cmd_unpack_objects(int argc, const char **argv, const char *prefix UNUSED) use(the_hash_algo->rawsz); /* Write the last part of the buffer to stdout */ - while (len) { - int ret = xwrite(1, buffer + offset, len); - if (ret <= 0) - break; - len -= ret; - offset += ret; - } + write_in_full(1, buffer + offset, len); /* All done */ return has_errors; From patchwork Sat Mar 2 19:03:47 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Junio C Hamano X-Patchwork-Id: 13579590 Received: from pb-smtp2.pobox.com (pb-smtp2.pobox.com [64.147.108.71]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 35A71405C1 for ; Sat, 2 Mar 2024 19:04:04 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=64.147.108.71 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1709406245; cv=none; b=MgvjmQzJ8HQxJhV7XepYzTzLxhZoIxRagZiUgaxLC2vrWjofUnFBAgn192CBemiYtpcSQ+sgTHlmAbieXVk9BUchEigTrNVAuhmaZTbKavpAOB5nlGg9Rz5RhVk4Vf0Y2CldMIp/6RV5y/oX1BDxVW6lqX8+0cgNhNj/TUsPhBE= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1709406245; c=relaxed/simple; bh=CwQVLntrmP1QgaXyDDBt9GN1JKuKD6IDErSQMiU3Lrg=; h=From:To:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=rrPzxviHjxIaD/WC3/JpyaPJ4pmogngl7AW4dSKtpmjiEaKD9TJmX1HCsr1frVTNLTk19cpZ4Y45jcEiQkaT7ZBzcWOqJ0wJtz0bYvba6dMO8eGpVi2xZFIxSVLDBNGv3VlBzoBdAc6hSI802T66TSDlgk2dpPExKTexa4ov5hQ= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=pobox.com; spf=pass smtp.mailfrom=pobox.com; dkim=pass (1024-bit key) header.d=pobox.com header.i=@pobox.com header.b=yH9dsOEU; arc=none smtp.client-ip=64.147.108.71 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=pobox.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pobox.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=pobox.com header.i=@pobox.com header.b="yH9dsOEU" Received: from pb-smtp2.pobox.com (unknown [127.0.0.1]) by pb-smtp2.pobox.com (Postfix) with ESMTP id 64A211E214B; Sat, 2 Mar 2024 14:03:57 -0500 (EST) (envelope-from gitster@pobox.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed; d=pobox.com; h=from:to :subject:date:message-id:in-reply-to:references:mime-version :content-transfer-encoding; s=sasl; bh=CwQVLntrmP1QgaXyDDBt9GN1J KuKD6IDErSQMiU3Lrg=; b=yH9dsOEUofdokYiZu/P2cu02lflMv0tymuR5ZlWPd 9T/a7FOWCFBV43vGOC7gfh6Yb98QbzBF3OTmn2Gzw1YjnnVL/SZaVNamgEeRSqMW icT+K49hUehpqafh/bym33YcPUZGLBoJjUoDDf7UwnvGhf1pFe+xUB7CHGnbOZZB Ok= Received: from pb-smtp2.nyi.icgroup.com (unknown [127.0.0.1]) by pb-smtp2.pobox.com (Postfix) with ESMTP id 5C1191E214A; Sat, 2 Mar 2024 14:03:57 -0500 (EST) (envelope-from gitster@pobox.com) Received: from pobox.com (unknown [34.125.176.30]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by pb-smtp2.pobox.com (Postfix) with ESMTPSA id BCD471E2148; Sat, 2 Mar 2024 14:03:56 -0500 (EST) (envelope-from gitster@pobox.com) From: Junio C Hamano To: git@vger.kernel.org Subject: [PATCH 2/3] sideband: avoid short write(2) Date: Sat, 2 Mar 2024 11:03:47 -0800 Message-ID: <20240302190348.3946569-3-gitster@pobox.com> X-Mailer: git-send-email 2.44.0-84-gb387623c12 In-Reply-To: <20240302190348.3946569-1-gitster@pobox.com> References: <20240302190348.3946569-1-gitster@pobox.com> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Pobox-Relay-ID: 9EB5E15C-D8C7-11EE-947D-25B3960A682E-77302942!pb-smtp2.pobox.com The sideband demultiplexor writes the data it receives on sideband with xwrite(). We can lose data if the underlying write(2) results in a short write. If they are limited to unimportant bytes like eye-candy progress meter, it may be OK to lose them, but lets be careful and ensure that we use write_in_full() instead. Note that the original does not check for errors, and this rewrite does not check for one. At least not yet. Signed-off-by: Junio C Hamano --- sideband.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sideband.c b/sideband.c index 266a67342b..5d8907151f 100644 --- a/sideband.c +++ b/sideband.c @@ -220,7 +220,7 @@ int demultiplex_sideband(const char *me, int status, } strbuf_addch(scratch, *brk); - xwrite(2, scratch->buf, scratch->len); + write_in_full(2, scratch->buf, scratch->len); strbuf_reset(scratch); b = brk + 1; @@ -247,7 +247,7 @@ int demultiplex_sideband(const char *me, int status, die("%s", scratch->buf); if (scratch->len) { strbuf_addch(scratch, '\n'); - xwrite(2, scratch->buf, scratch->len); + write_in_full(2, scratch->buf, scratch->len); } strbuf_release(scratch); return 1; From patchwork Sat Mar 2 19:03:48 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Junio C Hamano X-Patchwork-Id: 13579589 Received: from pb-smtp1.pobox.com (pb-smtp1.pobox.com [64.147.108.70]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 264C63FE54 for ; Sat, 2 Mar 2024 19:03:59 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=64.147.108.70 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1709406241; cv=none; b=aVTHlc76/qClRIqipI3D/3yechM9smKbdki0dFbBWYqF8cffRLFiuJPYcT0fidwLFcjRVhD7q4/aVtA6gn7D1/YarZVB4D/DD5zHq0oWXg5AiSE/GvCPAnIado/NQU4mZ9spEu1mlYVRm/qYOPHNDHRACFZcnp9l3O/eMC7TFiU= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1709406241; c=relaxed/simple; bh=XaOz0t95BPKfm/vULkguS+6VCx7JT12I9sJEbNBvkZQ=; h=From:To:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=Kkt1XXvM5TdHLpM8x4nEeHVwRJOmUMUuzCaQbl+ILuoMBg+YZtyS6PLfeS79zYV0NUAm2lNXC6mbLum0pSHwo5NUGhaPJ20PGlPnsOts+F8uVXPAqgczXny0PIe257drCD5UlC03L/MSawvabJzUbqyFFt4Wzo1lVXwTQYjHY80= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=pobox.com; spf=pass smtp.mailfrom=pobox.com; dkim=pass (1024-bit key) header.d=pobox.com header.i=@pobox.com header.b=xvNuewLm; arc=none smtp.client-ip=64.147.108.70 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=pobox.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pobox.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=pobox.com header.i=@pobox.com header.b="xvNuewLm" Received: from pb-smtp1.pobox.com (unknown [127.0.0.1]) by pb-smtp1.pobox.com (Postfix) with ESMTP id E18211E4D65; Sat, 2 Mar 2024 14:03:58 -0500 (EST) (envelope-from gitster@pobox.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed; d=pobox.com; h=from:to :subject:date:message-id:in-reply-to:references:mime-version :content-transfer-encoding; s=sasl; bh=XaOz0t95BPKfm/vULkguS+6VC x7JT12I9sJEbNBvkZQ=; b=xvNuewLmsJQ+fok/adYK/V0jAK1woM9wahGdVgdX4 tqGFc9g3rmd9NY7yeqaQdMfTOLqmm16naUWDZLVAvRyJgz5Q+m6OJOaF+JuAqE0J FQe/szjBcoEJvYMgwyn2N9vl8FGogALjmqGfjQta1zylJycD5nquxogMF7kAiW8Y wc= Received: from pb-smtp1.nyi.icgroup.com (unknown [127.0.0.1]) by pb-smtp1.pobox.com (Postfix) with ESMTP id D9C9F1E4D64; Sat, 2 Mar 2024 14:03:58 -0500 (EST) (envelope-from gitster@pobox.com) Received: from pobox.com (unknown [34.125.176.30]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by pb-smtp1.pobox.com (Postfix) with ESMTPSA id 50DBB1E4D63; Sat, 2 Mar 2024 14:03:58 -0500 (EST) (envelope-from gitster@pobox.com) From: Junio C Hamano To: git@vger.kernel.org Subject: [PATCH 3/3] repack: check error writing to pack-objects subprocess Date: Sat, 2 Mar 2024 11:03:48 -0800 Message-ID: <20240302190348.3946569-4-gitster@pobox.com> X-Mailer: git-send-email 2.44.0-84-gb387623c12 In-Reply-To: <20240302190348.3946569-1-gitster@pobox.com> References: <20240302190348.3946569-1-gitster@pobox.com> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Pobox-Relay-ID: 9FA407BA-D8C7-11EE-B984-78DCEB2EC81B-77302942!pb-smtp1.pobox.com When "git repack" repacks promisor objects, it starts a pack-objects subprocess and uses xwrite() to send object names over the pipe to it, but without any error checking. An I/O error or short write (even though a short write is unlikely for such a small amount of data) can result in a packfile that lacks certain objects we wanted to put in there, leading to a silent repository corruption. Use write_in_full(), instead of xwrite(), to mitigate short write risks, check errors from it, and abort if we see a failure. Signed-off-by: Junio C Hamano --- builtin/repack.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/builtin/repack.c b/builtin/repack.c index ede36328a3..15e4cccc45 100644 --- a/builtin/repack.c +++ b/builtin/repack.c @@ -314,8 +314,9 @@ static int write_oid(const struct object_id *oid, die(_("could not start pack-objects to repack promisor objects")); } - xwrite(cmd->in, oid_to_hex(oid), the_hash_algo->hexsz); - xwrite(cmd->in, "\n", 1); + if (write_in_full(cmd->in, oid_to_hex(oid), the_hash_algo->hexsz) < 0 || + write_in_full(cmd->in, "\n", 1) < 0) + die(_("failed to feed promisor objects to pack-objects")); return 0; }