Re: decmpfs_file_is_compressed changes?
Re: decmpfs_file_is_compressed changes?
- Subject: Re: decmpfs_file_is_compressed changes?
- From: Jorgen Lundman via Filesystem-dev <email@hidden>
- Date: Tue, 8 Oct 2019 16:39:50 +0900
- Autocrypt: addr=email@hidden; prefer-encrypt=mutual; keydata= mQGiBD2KhOgRBACHjaJI1Q5XudbpYACUeGAKFxsxhhg46svF6Y0HzzDP+ZMQENVGiqucma0H b/mY9knTJ5F+rQZO+LKG2T4oBH1VdznJkz28tkmZUXGCEmNOZ852Hb1fFRwnbngRylMq7cEV TgupfCWYbC6Qw7eKBlHWjlvFosn/r828j7STe0CtawCgsbMU6598cmwIvOXaSGuDHLnMzfED /0SVpaTDADGymmjZ/RUyl7wc0D/1Dd0N17kZzQuwXQtj0CWnGwC8XWEvYLdAwNoGmzkJgVg4 3l5JPo4Yeuj7hNayRCjQU6jvnSdj0AuItfuE76/YdSG/X+TWAdHsizXtdXxf6KrS5PSjWult b4yL2EoYkiN58SflRa287mwJxatpA/9/axDaElD8Ko4QsskT2FamKLms1clT8IcuN1JNP71h b4gG0sng8jIl534r3KucJjlYEIp6lL3jBD/lcNQqd+UXMsR7KKEuDvW/ZMzQdLMvpyRCN463 jzrclxG1rXuvyeSWKiOnQbcD5pwsff3xlbb3mMovkf6rbXxvZxQlfUAuRLQkSm9yZ2VuIEx1 bmRtYW4gPGx1bmRtYW5AbHVuZG1hbi5uZXQ+iFkEExECABkFAj2KhOgECwcDAgMVAgMDFgIB Ah4BAheAAAoJEPNmEmurHstLXGgAnA7x3Ipz89W9tWjeJZ3AeEF6DAX/AJ4+QReCt51Nnovp 3T17+gy+p91MEbkBDQQ9ioTpEAQA5omoMw1PE/2reQ96uhFr/eFtGEc6136juCE6Lbrrc52j /DwsB0s87miNKfBRUGKXaqLQmfPY9Qajz/MHUHQ9iOkpCGou8RNviJGKFup4qf/g1qzNQ9ud pgkTEc7qZ7qBWG5HcOoIAvM78qvKwV0Sedry6TpOaiWsuL1HWdSGXI8AAwcEALVHV3MQ+a5K vwhX8Pam52AnIEzcw3m7tcSoJikLZhR//spUaZ3++hN+NNlSxgnGJ+VWKyhxc+SA/IO32vFP rAS6HxyfdZtIKPXfwM/8HjTUa4n6DR8ChIrS43X6cz3TZCFVD9tYLLP5cKo6uuPBd807EQN+ GJAzER048LeRiWMNiEYEGBECAAYFAj2KhOoACgkQ82YSa6sey0soMgCgg7zP7pYVB12AgkyN 0aUsovPMPCQAnjaw8yZSKLeBOzplkeFSIIbfWqTs
- Dkim-filter: OpenDKIM Filter v2.10.3 mail.lundman.net 9319711C0CD
- Openpgp: preference=signencrypt
Ok, turns out the change is in "struct decmpfs" and not the function calls.
Two new fields were added "nchildren, total_size" according to lldb (types
unknown).
Which ordinarily would not cause trouble if we were using
decmpfs_cnode_alloc() to allocate the node, but decmpfs_cnode_alloc() was
not added until 10.12 so we shadowed the struct and used internal allocs
for all versions.
I have now flipped it around so that OSX versions 10.11 and older, uses a
shadow version of the struct with internal allocs, and 10.12 onward use
decmpfs_cnode_alloc(). Should take care of any future changes.
It also answers why it always wrote the same values - as lck_rw_t is the
final member of the struct, and lck_rw_init() would reset it.
Cheers,
Lund
Jorgen Lundman via Filesystem-dev wrote:
>
> Hello list!
>
> I was hoping to ask about any changes to decmpfs_file_is_compressed() in
> Catalina as I've just spent 3 weeks chasing down a problem with random
> memory corruption.
>
> The stack looks like:
>
> verify_events(NULL);
> spl_decmpfs_cnode_init(cp);
> verify_events(NULL);
> /* _is_compressed() will getattr to make sure UF_COMPRESSED */
> iscompressed = spl_decmpfs_file_is_compressed(ap->a_vp, cp);
> ~~~
> _decmpfs_file_is_compressed + 0x263
> _vnode_getattr + 0xae
> zfs_vnop_getattr + 0x24
> ~~~
> verify_events(NULL);
>
> (lldb) if (curr == (void *)0x0000200721000000ULL)
> ==> panic("triggered");
>
>
> The repeated call to `verify_events()` runs through all lists, and memory,
> to check for corruption. The call just before
> `decmpfs_file_is_compressed()` succeeds.
>
> `decmpfs_file_is_compressed()` then calls `vnode_getattr()` to confirm
> UF_COMPRESSED is set, and at the very start of our getattr
> `zfs_vnop_getattr()` we call `verify_events()` and detect memory corruption.
>
> Interestingly, it is always writing the value 0x0000200721000000 at a
> (pseudo) random location.
>
> Usually, I would simply check the sources for xnu-6153.0.139.161.2 - but I
> don't think they are available yet. But perhaps there were changes to the
> number/type of arguments it takes? It would be nice to be able to fix my
> side before Catalina going public.
>
>
>
> History (Feel free to skip):
>
> * We do not advertise VOL_CAP_FMT_DECMPFS_COMPRESSION but unfortunately,
> the AppleFSCompression library (as used by many Applications like 'Mail'
> etc) totally ignores the lack of this property, and will attempt/write
> decmpfs anyway.
>
> * If we try to stop it, by returning error to setattr(UF_COMPRESSED) it
> ignores the errors, and keeps going
>
> * If we try to stop it, by returning error on XATTR create/write of
> "com.apple.decmpfs" or "com.apple.ResourceFork" - it ignores the error, and
> keeps going
>
> * Then it truncates the file to 0. "Losing" the file data.
>
> Currently, we handle this by playing-along with userland, but as soon as
> the file is truncated it calls setattr(UF_COMPRESSED).
>
> We then decmpfs decompress the file data back, and delete the xattr data.
>
> Since the compression level is a user defined property for our filesystem,
> and users can set it to disabled, lz4, gzip-9 and so on. So both to honour
> the users' choice, and that some compression types have better performance.
>
> This is rather frustrating, especially since there is a
> VOL_CAP_FMT_DECMPFS_COMPRESSION property. (It is somewhat alarming that all
> errors are just ignored as well) It forces us to support decmpfs, even if
> we don't want to.
>
> We felt it best to use XNU's built-in decmpfs, to avoid code duplication
> and possible future case of Apple updating/changing it.
>
> Unfortunately, all decmpfs function calls are not exported, so we aren't
> really allowed to use them - we just do anyway - so that is a risk I took
> and that's on me, it didn't work out this time.
>
> So perhaps the only answer is to copy/paste the decmpfs code over :(
>
> Lund
>
--
Jorgen Lundman | <email@hidden>
Unix Administrator | +81 (0)90-5578-8500
Shibuya-ku, Tokyo | Japan
_______________________________________________
Do not post admin requests to the list. They will be ignored.
Filesystem-dev mailing list (email@hidden)
Help/Unsubscribe/Update your Subscription:
This email sent to email@hidden