{"thread":{"id":"57460","subject":"BUG: fast-import, ftruncate, and file mode","startedAt":"2022-02-23T07:48:17Z","lastAt":"2022-02-23T23:18:24Z","messageCount":4,"participants":["Elijah Newren","Phillip Wood","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"449227","messageId":"CABPp-BERVCynOVvBq0QL49Ah+gy3W2snUVWBHfzXaVpXX3Dpyg@mail.gmail.com","threadId":"57460","inReplyTo":null,"subject":"BUG: fast-import, ftruncate, and file mode","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2022-02-23T07:47:53Z","receivedAt":"2022-02-23T07:48:17Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi,\n\nfast-import makes use of odb_mkstemp(), which creates a secure\ntemporary file and opens it with mode 0444, and then uses it for its\npackfile writing. Sometimes, fast-import will call its\ntruncate_pack() function, which makes use of ftruncate().\n\nAccording to my local manpage, \"With ftruncate(), the file must be\nopen for writing; with truncate(), the file must be writable.\"\n\nThe writable requirement does not appear to be enforced by the kernel\non common filesystems like ext4 or zfs, but this is enforced on some\nfilesystems. Apparently a \"VxFS Veritas filesystem\" got triggered by\nthis...and some helpful bug reporters tracked this problem down and\nfound a workaround (for the filter-repo usecase, they recompiled a\nspecial copy of git using mode 0644 for odb_mkstemp, since it was just\nan intermediate step anyway and won't be used elsewhere).\n\nI'm not sure if we should (1) just stop calling truncate_pack() in\nfast-import (it's merely an optimization), or (2) modify odb_mkstemp()\nto allow specifying a mode and also have something like\nfinalize_object_file() modify the mode before renaming, or (3) if we\nshould do something else entirely. Someone more familiar with the\nobject storage side of things probably knows. But I'm guessing that\nonce someone who knows the area states what should be done, that this\nmight be a small micro-project suitable for the GSoC interns (or\nanyone else wanting to get involved)?\n\nOriginal details from here:\nhttps://github.com/newren/git-filter-repo/issues/342#issuecomment-1047638304\n"},{"id":"449249","messageId":"3bdff4ba-fb5f-e369-306d-5510ab20893a@gmail.com","threadId":"57460","inReplyTo":"CABPp-BERVCynOVvBq0QL49Ah+gy3W2snUVWBHfzXaVpXX3Dpyg@mail.gmail.com","subject":"Re: BUG: fast-import, ftruncate, and file mode","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2022-02-23T13:59:10Z","receivedAt":"2022-02-23T13:59:21Z","isPatch":false,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 23/02/2022 07:47, Elijah Newren wrote:\n> Hi,\n> \n> fast-import makes use of odb_mkstemp(), which creates a secure\n> temporary file and opens it with mode 0444, and then uses it for its\n> packfile writing. Sometimes, fast-import will call its\n> truncate_pack() function, which makes use of ftruncate().\n> \n> According to my local manpage, \"With ftruncate(), the file must be\n> open for writing; with truncate(), the file must be writable.\"\n> \n> The writable requirement does not appear to be enforced by the kernel\n> on common filesystems like ext4 or zfs, but this is enforced on some\n> filesystems. Apparently a \"VxFS Veritas filesystem\" got triggered by\n> this...and some helpful bug reporters tracked this problem down and\n> found a workaround (for the filter-repo usecase, they recompiled a\n> special copy of git using mode 0644 for odb_mkstemp, since it was just\n> an intermediate step anyway and won't be used elsewhere).\n\nAm I missing something or is this really a file system bug? Surely if we \nhave opened a file for writing the file permissions when we call \nftruncate() should be irrelevant?\n\nBest Wishes\n\nPhillip\n\n> I'm not sure if we should (1) just stop calling truncate_pack() in\n> fast-import (it's merely an optimization), or (2) modify odb_mkstemp()\n> to allow specifying a mode and also have something like\n> finalize_object_file() modify the mode before renaming, or (3) if we\n> should do something else entirely. Someone more familiar with the\n> object storage side of things probably knows. But I'm guessing that\n> once someone who knows the area states what should be done, that this\n> might be a small micro-project suitable for the GSoC interns (or\n> anyone else wanting to get involved)?\n> \n> Original details from here:\n> https://github.com/newren/git-filter-repo/issues/342#issuecomment-1047638304\n\n"},{"id":"449269","messageId":"CABPp-BF_ygATPVGfSR24URm5ZTHBJwJd0miMtgMgNWfw_o33Nw@mail.gmail.com","threadId":"57460","inReplyTo":"3bdff4ba-fb5f-e369-306d-5510ab20893a@gmail.com","subject":"Re: BUG: fast-import, ftruncate, and file mode","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2022-02-23T15:33:29Z","receivedAt":"2022-02-23T15:33:44Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Wed, Feb 23, 2022 at 5:59 AM Phillip Wood wrote:\n>\n> On 23/02/2022 07:47, Elijah Newren wrote:\n> > Hi,\n> >\n> > fast-import makes use of odb_mkstemp(), which creates a secure\n> > temporary file and opens it with mode 0444, and then uses it for its\n> > packfile writing. Sometimes, fast-import will call its\n> > truncate_pack() function, which makes use of ftruncate().\n> >\n> > According to my local manpage, \"With ftruncate(), the file must be\n> > open for writing; with truncate(), the file must be writable.\"\n> >\n> > The writable requirement does not appear to be enforced by the kernel\n> > on common filesystems like ext4 or zfs, but this is enforced on some\n> > filesystems. Apparently a \"VxFS Veritas filesystem\" got triggered by\n> > this...and some helpful bug reporters tracked this problem down and\n> > found a workaround (for the filter-repo usecase, they recompiled a\n> > special copy of git using mode 0644 for odb_mkstemp, since it was just\n> > an intermediate step anyway and won't be used elsewhere).\n>\n> Am I missing something or is this really a file system bug? Surely if we\n> have opened a file for writing the file permissions when we call\n> ftruncate() should be irrelevant?\n>\n> Best Wishes\n>\n> Phillip\n\nOh, indeed, looks like I can't read late at night. Sorry for the noise.\n"},{"id":"449366","messageId":"xmqqo82xp3ye.fsf@gitster.g","threadId":"57460","inReplyTo":"CABPp-BF_ygATPVGfSR24URm5ZTHBJwJd0miMtgMgNWfw_o33Nw@mail.gmail.com","subject":"Re: BUG: fast-import, ftruncate, and file mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-02-23T23:18:17Z","receivedAt":"2022-02-23T23:18:24Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elijah Newren writes:\n\n>> Am I missing something or is this really a file system bug? Surely if we\n>> have opened a file for writing the file permissions when we call\n>> ftruncate() should be irrelevant?\n>> ...\n> Oh, indeed, looks like I can't read late at night. Sorry for the noise.\n\nWas wondering the same thing.\n\nWe open for writing, write one object at a time and when we realize\nwe do not have to, we rewind that stream we are writing into using\nftruncate(). It would be a serious problem if we cannot do that.\n"}]}