threads / bug / 57460

BUG: fast-import, ftruncate, and file mode

Subject: BUG: fast-import, ftruncate, and file mode

## tl;dr

4 messages between Feb 23, 2022 and Feb 23, 2022.

replies: 3people: 3as markdown or json

Elijah Newren· Feb 23, 2022, 07:47 UTC · lore
Hi,

fast-import makes use of odb_mkstemp(), which creates a secure temporary file and opens it with mode 0444, and then uses it for its packfile writing. Sometimes, fast-import will call its truncate_pack() function, which makes use of ftruncate().

According to my local manpage, "With ftruncate(), the file must be open for writing; with truncate(), the file must be writable."

The writable requirement does not appear to be enforced by the kernel on common filesystems like ext4 or zfs, but this is enforced on some filesystems. Apparently a "VxFS Veritas filesystem" got triggered by this...and some helpful bug reporters tracked this problem down and found a workaround (for the filter-repo usecase, they recompiled a special copy of git using mode 0644 for odb_mkstemp, since it was just an intermediate step anyway and won't be used elsewhere).

I'm not sure if we should (1) just stop calling truncate_pack() in fast-import (it's merely an optimization), or (2) modify odb_mkstemp() to allow specifying a mode and also have something like finalize_object_file() modify the mode before renaming, or (3) if we should do something else entirely. Someone more familiar with the object storage side of things probably knows. But I'm guessing that once someone who knows the area states what should be done, that this might be a small micro-project suitable for the GSoC interns (or anyone else wanting to get involved)?

Original details from here: https://github.com/newren/git-filter-repo/issues/342#issuecomment-1047638304

Phillip Wood· Feb 23, 2022, 13:59 UTC · re: Elijah Newren · lore

Re: BUG: fast-import, ftruncate, and file mode

On 23/02/2022 07:47, Elijah Newren wrote:
Show 17 quoted lines
> Hi,
> 
> fast-import makes use of odb_mkstemp(), which creates a secure
> temporary file and opens it with mode 0444, and then uses it for its
> packfile writing.  Sometimes, fast-import will call its
> truncate_pack() function, which makes use of ftruncate().
> 
> According to my local manpage, "With ftruncate(), the file must be
> open for writing; with truncate(), the file must be writable."
> 
> The writable requirement does not appear to be enforced by the kernel
> on common filesystems like ext4 or zfs, but this is enforced on some
> filesystems.  Apparently a "VxFS Veritas filesystem" got triggered by
> this...and some helpful bug reporters tracked this problem down and
> found a workaround (for the filter-repo usecase, they recompiled a
> special copy of git using mode 0644 for odb_mkstemp, since it was just
> an intermediate step anyway and won't be used elsewhere).

Am I missing something or is this really a file system bug? Surely if we have opened a file for writing the file permissions when we call ftruncate() should be irrelevant?

Best Wishes
Phillip
Show 12 quoted lines
> I'm not sure if we should (1) just stop calling truncate_pack() in
> fast-import (it's merely an optimization), or (2) modify odb_mkstemp()
> to allow specifying a mode and also have something like
> finalize_object_file() modify the mode before renaming, or (3) if we
> should do something else entirely.  Someone more familiar with the
> object storage side of things probably knows.  But I'm guessing that
> once someone who knows the area states what should be done, that this
> might be a small micro-project suitable for the GSoC interns (or
> anyone else wanting to get involved)?
> 
> Original details from here:
> https://github.com/newren/git-filter-repo/issues/342#issuecomment-1047638304
Elijah Newren· Feb 23, 2022, 15:33 UTC · re: Phillip Wood · lore

Re: BUG: fast-import, ftruncate, and file mode

On Wed, Feb 23, 2022 at 5:59 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
Show 27 quoted lines
>
> On 23/02/2022 07:47, Elijah Newren wrote:
> > Hi,
> >
> > fast-import makes use of odb_mkstemp(), which creates a secure
> > temporary file and opens it with mode 0444, and then uses it for its
> > packfile writing.  Sometimes, fast-import will call its
> > truncate_pack() function, which makes use of ftruncate().
> >
> > According to my local manpage, "With ftruncate(), the file must be
> > open for writing; with truncate(), the file must be writable."
> >
> > The writable requirement does not appear to be enforced by the kernel
> > on common filesystems like ext4 or zfs, but this is enforced on some
> > filesystems.  Apparently a "VxFS Veritas filesystem" got triggered by
> > this...and some helpful bug reporters tracked this problem down and
> > found a workaround (for the filter-repo usecase, they recompiled a
> > special copy of git using mode 0644 for odb_mkstemp, since it was just
> > an intermediate step anyway and won't be used elsewhere).
>
> Am I missing something or is this really a file system bug? Surely if we
> have opened a file for writing the file permissions when we call
> ftruncate() should be irrelevant?
>
> Best Wishes
>
> Phillip
Oh, indeed, looks like I can't read late at night.  Sorry for the noise.
Junio C Hamano· Feb 23, 2022, 23:18 UTC · re: Elijah Newren · lore

Re: BUG: fast-import, ftruncate, and file mode

Elijah Newren <newren@gmail.com> writes:
Show 5 quoted lines
>> Am I missing something or is this really a file system bug? Surely if we
>> have opened a file for writing the file permissions when we call
>> ftruncate() should be irrelevant?
>> ...
> Oh, indeed, looks like I can't read late at night.  Sorry for the noise.
Was wondering the same thing.

We open for writing, write one object at a time and when we realize we do not have to, we rewind that stream we are writing into using ftruncate(). It would be a serious problem if we cannot do that.

← back to recent threads