On Tue, 2025-01-14 at 23:13 +0100, Antoine Viallon wrote:
> Hello Viacheslav,
> thanks for your review.
>
> On 14/01/2025 20:17, Viacheslav Dubeyko wrote:
>
> > Does it exist any way to reproduce the issue in stable manner?
> > Could
> > you please share any steps to repeat it? It will be great to have
> > this
> > description in the patch comment.
>
> This issue can probably be reproduced by having a CephFS
> subvolumegroup
> be mounted to a kernel Ceph client (version 6.12.8), where the auth
> credentials are scoped to a specific path:
>
> client.services:
> key: REDACTED
> caps: [mds] allow rw fsname=cephfs path=/volumes/
> caps: [mon] allow r fsname=cephfs
> caps: [osd] allow rw tag cephfs data=cephfs
>
> Then, you simply need to trigger a lot of file permission check
> (either
> by using the FS for long or doing LOSF I/O). This could be probably
> be
> done by doing:
>
> seq 1 100000 | xargs -P32 --replace touch
> /path/to/your/cephfs/mount/file-{}
> seq 1 100000 | xargs -P32 --replace cat
> /path/to/your/cephfs/mount/file-{}
>
> The idea is to place yourself in a situation where the target path
> being
> matched by ceph_mds_auth_match does not contain your credential
> (auth)
> path AT ALL. This can be done when mounting a subvolumegroup, for
> instance:
>
>
> services@00000000-0000-0000-0000-000000000000.cephfs=/volumes/contain
> ers ceph rw,noatime,name=services,secret=<hidden>,ms_mode=prefer-
> crc,mount_timeout=300,acl,mon_addr=[REDACTED]
>
Thanks for the explanation. This description is valuable to understand
the issue nature and the path to reproduce it. It really needs to be in
patch description.
> Since you never enter
> https://elixir.bootlin.com/linux/v6.13-rc3/source/fs/ceph/mds_client.c#L5697
>
> or
> https://elixir.bootlin.com/linux/v6.13-rc3/source/fs/ceph/mds_client.c#L5703
> ,
> you never free _tpath (whatever free_tpath's value might be).
>
> > As far as I can see, we have several kfree() calls in the logic of
> > this
> > method:
> > (1)
> > https://elixir.bootlin.com/linux/v6.13-rc3/source/fs/ceph/mds_client.c#L5697
> > (2)
> > https://elixir.bootlin.com/linux/v6.13-rc3/source/fs/ceph/mds_client.c#L5703
> >
> > And you are adding the third call. I believe that it will be much
> > cleaner solution if we have only one kfree() call and goto from all
> > other places. Could you please rework your fix?
>
> I absolutely agree, and was thinking the same thing. I'll rework my
> patch to simplify this kfree path.
>
Sounds great!
Thanks,
Slava.
