All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Patrick Steinhardt <ps@pks.im>
Cc: git@vger.kernel.org,  "Randall S. Becker" <randall.becker@nexbridge.ca>
Subject: Re: [PATCH 2/2] reftable/stack: accept insecure random bytes
Date: Wed, 08 Jan 2025 09:40:58 -0800	[thread overview]
Message-ID: <xmqqo70hmcet.fsf@gitster.g> (raw)
In-Reply-To: <Z36l--QUjaYYb6Uf@pks.im> (Patrick Steinhardt's message of "Wed, 8 Jan 2025 17:21:15 +0100")

Patrick Steinhardt <ps@pks.im> writes:

> Hm. The problem is when Git dies in the middle of a transaction:
>
>   1. We write the temporary table.
>   2. We compute the not-so-random suffix.
>   3. We write the temporary "tables.list" file.
>   4. We move the temporary table into place using the not-so-random
>      suffix.
>   5. Git dies before updating "tables.list".
>
> Now we have the temporary table moved into place, but "tables.list"
> hasn't been updated yet. When the next Git process comes along and wants
> to update the table it would result in an error if it computed the same
> suffix.

Here, I hear that we _do_ depend on the suffix being relatively
unique.  Once our random number generator decides to give the same
number twice to cause collision, the reftable data gets corrupt?

> The reftable library knows to clean up such stale tables when not
> referenced by the "tables.list" file, but it doesn't do so on every
> write. So this would likely still cause issues in practice.
>
> I already though about this scenario when writing my mail, but didn't
> really think about it as "correctness". But I guess it is.

Hmph.  I am not sure how I should feel about this.  Our reliance on
hash functions (which can be made to collide) not colliding is one
thing, but is it sensibly safe to rely on a cryptographically
unpredictable random generator not to yield the same suffix twice
during the lifetime of an previous invocation for correctness?

  reply	other threads:[~2025-01-08 17:41 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-01-07 15:26 [PATCH 0/2] reftable/stack: stop dying on exhausted entropy pool Patrick Steinhardt
2025-01-07 15:26 ` [PATCH 1/2] wrapper: allow generating insecure random bytes Patrick Steinhardt
2025-01-07 15:27 ` [PATCH 2/2] reftable/stack: accept " Patrick Steinhardt
2025-01-07 15:37   ` rsbecker
2025-01-07 20:56   ` Junio C Hamano
2025-01-07 21:03     ` rsbecker
2025-01-07 21:09       ` Junio C Hamano
2025-01-07 21:03     ` Junio C Hamano
2025-01-08  6:51       ` Patrick Steinhardt
2025-01-08 15:39         ` Junio C Hamano
2025-01-08 16:21           ` Patrick Steinhardt
2025-01-08 17:40             ` Junio C Hamano [this message]
2025-01-08 18:16               ` Patrick Steinhardt
2025-01-07 23:56   ` rsbecker
2025-01-07 23:21 ` [PATCH 0/2] reftable/stack: stop dying on exhausted entropy pool brian m. carlson
2025-01-07 23:54   ` rsbecker
2025-01-08  7:18     ` Patrick Steinhardt
2025-01-08 13:50       ` rsbecker
2025-01-08 22:44       ` brian m. carlson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqo70hmcet.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=ps@pks.im \
    --cc=randall.becker@nexbridge.ca \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.