Bug#995159: hfsprogs: Directory hardlink problems cannot be repaired.

[John Paul Adrian Glaubitz]

Hi Daniel!

Apologies for the late reply. I had seen your message back then but it
somehow fell of the table and eventually forgot to answer. I just now
found your report again since I resumed working on the hfsprogs package.

Apple (the upstream) never finished the implementation of the directory hardlink support
in fsck_hfs. Since, once again, my TimeMachine backup rendered unrepairable due to directory
hardlink errors, I implemented what was required to fix the errors I encountered.

The current hfsprogs package in Debian is based on a patched version 540.1 of Apple's
»diskdev_cmds« package [1] while Apple has actually continued their work on the HFS code.
in that package. However, after version 557.3.1 Apple decided to move the HFS code into a
separate package called just »hfs« [2] which makes sense given the fact that Apple switched
their default filesystem to APFS instead and diskdev_cms is no longer strictly tied to HFS.

I have started working on an updated Debian hfsprogs package which is based on the latest version
556.100.11 of the hfs package [3]. The code already compiles in openSUSE [4] but I don't consider
the Linux port to be complete yet.

However, you may want to try whether the latest version of Apple's own fsck_hfs has support for
fixing hardlink problems. If not, you may want to rebase your patch on top of hfs 560.100.11.

My trust in Apple's feedback system is very low so I am sending the patch to you in the hopes
that you can forward it to Apple.

Try opening a pull request on Github [5]. Apple has actually moved their open source projects to
Github and it seems they're actually accepting pull requests for some projects.

The patch I added should work, with offsets, on both, Debian's sources as well as Apple's sources.
- Any hardlink inode with a parent other than the metadata directory is changed
to have the correct parent.
I have seen these. I can understand how this happens when a normal directory
is hardlined for the first time. The journal SHOULD prevent this from being a
problem but obviously this is not the case.
- Directory hardlink inodes named "temp..." get deleted.
These directories were deleted while open when the volume was removed without
unmounting. Since these directories have already been deleted, deleting them
in fsck is the right thing to do. (Documented like that for files in the HFS+
spec.)
- Directory hardlink inodes with names other than "temp..." or "dir_..." are moved
to "lost+found".
It would be better to rename them to "dir_..." in a first pass, there might still
be valid links to them. These links are deleted by a different repair step and I
did not see how to remove those other repair steps. Would probably lead to a bigger
rewrite of the hardlink check code so I decided to just move these to l+f.
I have never seen this anyways.
After all, none of the errors should happen in a journaled filesystem (directory
hardlinks require the Journal to be active). My guess is that using HFS+ over AFP
(as TimeMachine does) does not correctly persist the journal before persisting changes.
Thus loosing the connection during directory hardlink operation can result in partially
executed atomic operation.

Sounds like a very useful and welcome improvement. I will definitely incorporate this
patch once I'm ready with the new upstream version. But maybe we're lucky and you can
get your changes merged upstream with Apple in the mean time.

FWIW, I have also reached out to Apple Legal trying to convince them to relicense the
hfs package to a more permissive license such as the Apache License like they have done
for other projects such as mDNSResponder [6]. Apple's own APSL license is considered
non-free by Debian [7] which makes redistribution of the package more complicated.

I will attach the patch and test cases for each of the three fixed errors. These test
cases have been constructed in a hex editor, they might not be what a real error looks
like. The first two repairs successfully repaired my broken backup file so I guess
they work (sometimes).

Thanks, having test cases will be very useful for verifying my patched hfs package!

Adrian

[1] https://github.com/apple-opensource/diskdev_cmds/tree/540.1
[2] https://github.com/apple-opensource/hfs
[3] https://github.com/glaubitz/hfs/tree/linux
[4] https://build.opensuse.org/package/show/home:glaubitz:branches:filesystems/hfsplus-tools
[5] https://github.com/apple-opensource/hfs
[6] https://github.com/apple-oss-distributions/mDNSResponder/
[7] https://wiki.debian.org/DFSGLicenses#Apple_Public_Source_License_.28APSL.29

[Daniel Höpfl]

Hi Adrian,

Your answer is still faster than anything from Apple.

Just note that your links go to an unofficial repo.
The official repository is at
<https://github.com/apple-oss-distributions/hfs>.

HFS does not look like one of the projects that care about pull
requests:
<https://github.com/apple-oss-distributions/hfs/pulls>

Bye,
Daniel

Post by John Paul Adrian Glaubitz
Hi Daniel!
Apologies for the late reply. I had seen your message back then but it
somehow fell of the table and eventually forgot to answer. I just now
found your report again since I resumed working on the hfsprogs package.

Apple (the upstream) never finished the implementation of the
directory hardlink support
in fsck_hfs. Since, once again, my TimeMachine backup rendered
unrepairable due to directory
hardlink errors, I implemented what was required to fix the errors I encountered.

The current hfsprogs package in Debian is based on a patched version 540.1 of Apple's
»diskdev_cmds« package [1] while Apple has actually continued their work on the HFS code.
in that package. However, after version 557.3.1 Apple decided to move the HFS code into a
separate package called just »hfs« [2] which makes sense given the fact
that Apple switched
their default filesystem to APFS instead and diskdev_cms is no longer strictly tied to HFS.
I have started working on an updated Debian hfsprogs package which is
based on the latest version
556.100.11 of the hfs package [3]. The code already compiles in
openSUSE [4] but I don't consider
the Linux port to be complete yet.
However, you may want to try whether the latest version of Apple's own
fsck_hfs has support for
fixing hardlink problems. If not, you may want to rebase your patch on
top of hfs 560.100.11.

My trust in Apple's feedback system is very low so I am sending the
patch to you in the hopes
that you can forward it to Apple.

Try opening a pull request on Github [5]. Apple has actually moved
their open source projects to
Github and it seems they're actually accepting pull requests for some projects.

The patch I added should work, with offsets, on both, Debian's sources
as well as Apple's sources.
- Any hardlink inode with a parent other than the metadata directory is changed
to have the correct parent.
I have seen these. I can understand how this happens when a normal directory
is hardlined for the first time. The journal SHOULD prevent this from being a
problem but obviously this is not the case.
- Directory hardlink inodes named "temp..." get deleted.
These directories were deleted while open when the volume was removed without
unmounting. Since these directories have already been deleted, deleting them
in fsck is the right thing to do. (Documented like that for files in the HFS+
spec.)
- Directory hardlink inodes with names other than "temp..." or "dir_..." are moved
to "lost+found".
It would be better to rename them to "dir_..." in a first pass, there might still
be valid links to them. These links are deleted by a different repair step and I
did not see how to remove those other repair steps. Would probably lead to a bigger
rewrite of the hardlink check code so I decided to just move these to l+f.
I have never seen this anyways.
After all, none of the errors should happen in a journaled filesystem (directory
hardlinks require the Journal to be active). My guess is that using HFS+ over AFP
(as TimeMachine does) does not correctly persist the journal before persisting changes.
Thus loosing the connection during directory hardlink operation can result in partially
executed atomic operation.

Sounds like a very useful and welcome improvement. I will definitely incorporate this
patch once I'm ready with the new upstream version. But maybe we're lucky and you can
get your changes merged upstream with Apple in the mean time.
FWIW, I have also reached out to Apple Legal trying to convince them to relicense the
hfs package to a more permissive license such as the Apache License like they have done
for other projects such as mDNSResponder [6]. Apple's own APSL license is considered
non-free by Debian [7] which makes redistribution of the package more complicated.

I will attach the patch and test cases for each of the three fixed errors. These test
cases have been constructed in a hex editor, they might not be what a real error looks
like. The first two repairs successfully repaired my broken backup file so I guess
they work (sometimes).

Thanks, having test cases will be very useful for verifying my patched hfs package!
Adrian

[1] https://github.com/apple-opensource/diskdev_cmds/tree/540.1
[2] https://github.com/apple-opensource/hfs
[3] https://github.com/glaubitz/hfs/tree/linux
[4]
https://build.opensuse.org/package/show/home:glaubitz:branches:filesystems/hfsplus-tools
[5] https://github.com/apple-opensource/hfs
[6] https://github.com/apple-oss-distributions/mDNSResponder/
[7]
https://wiki.debian.org/DFSGLicenses#Apple_Public_Source_License_.28APSL.29

[John Paul Adrian Glaubitz]

Hi Daniel!

Post by Daniel Höpfl
Your answer is still faster than anything from Apple.

Took me still way too long.

Post by Daniel Höpfl
Just note that your links go to an unofficial repo.
The official repository is at
<https://github.com/apple-oss-distributions/hfs>.

You're absolutely right and I only noticed this after sending the reply
to your bug report. I will have to rebase my Linux patches.

Post by Daniel Höpfl
HFS does not look like one of the projects that care about pull
<https://github.com/apple-oss-distributions/hfs/pulls>

I saw your pull request and I think that Apple is simply not monitoring
this repository. I am trying to find someone at Apple that can help us
here, both with the license and community improvements. It certainly
won't be easy though.

Adrian