Re: [PATCH 0/3] ceph: minor fixes and encrypted snapshot names

[Luís Henriques]

Luís Henriques <lhenriques@xxxxxxx> writes:

> Hi!
>
> I'm sending another iteration of the encrypted snapshot names patch.  This
> patch assumes PR#45224 [1] to be merged as it adds support for the
> alternate names.
>
> Two notes:
>
> 1. Patch 0001 is just a small fix from another fscrypt patch.  It's
>    probably better to simply squash it.
>
> 2. I'm not sure how easy it is to hit the UAF fixed by patch 0002.  I can
>    reproduce it easily by commenting the code that adds the
>    DCACHE_NOKEY_NAME flag in patch 0003.

Obviously, immediately after sending this patchset I realized I failed to
mention a very (*VERY*) important note:

Snapshot names can not start with a '_'.  I think the reason is related
with the 'long snapshot names', but I can't really remember the details
anymore.  The point is that an encrypted snapshot name base64-encoded
*may* end-up starting with an '_' as we're using the base64-url variant.

I really don't know if it's possible to fix that.  I guess that in that
case the user will get an error and fail to create the snapshot but he'll
be clueless because the reason.  Probably a warning can be added to the
kernel logs, but maybe there are other ideas.

Cheers,
-- 
Luís


> Any comments are welcome (including for the PR mentioned above, of course).
>
> [1] https://github.com/ceph/ceph/pull/45224
>
> Luís Henriques (3):
>   ceph: fix error path in ceph_readdir()
>   ceph: fix use-after-free in ceph_readdir
>   ceph: add support for encrypted snapshot names
>
>  fs/ceph/dir.c   | 11 ++++++++++-
>  fs/ceph/inode.c | 13 +++++++++++++
>  2 files changed, 23 insertions(+), 1 deletion(-)
>