linux-kernel - Re: [PATCH 06/18] fsinfo: Add a uniquifier ID to struct mount [ver #21]

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 05 Aug 2020 15:13:48 +0100
From:   David Howells <dhowells@...hat.com>
To:     Miklos Szeredi <miklos@...redi.hu>
Cc:     dhowells@...hat.com, viro@...iv.linux.org.uk,
        torvalds@...ux-foundation.org, raven@...maw.net,
        mszeredi@...hat.com, christian@...uner.io, jannh@...gle.com,
        darrick.wong@...cle.com, kzak@...hat.com, jlayton@...hat.com,
        linux-api@...r.kernel.org, linux-fsdevel@...r.kernel.org,
        linux-security-module@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 06/18] fsinfo: Add a uniquifier ID to struct mount [ver #21]

Miklos Szeredi <miklos@...redi.hu> wrote:

> > +#ifdef CONFIG_FSINFO
> > +	u64	mnt_unique_id;		/* ID unique over lifetime of kernel */
> > +#endif
>
> Not sure if it's worth making conditional.

You can't get at it without CONFIG_FSINFO=y as it stands, but making it
unconditional might be reasonable.

> > -		n.auxiliary_mount	= aux->mnt_id;
> > +		n.auxiliary_mount = aux->mnt_unique_id;
>
> Hmm, so we now have two ID's:
>
>  - one can be used to look up the mount
>  - one is guaranteed to be unique
>
> With this change the mount cannot be looked up with FSINFO_FLAGS_QUERY_MOUNT,
> right?
>
> Should we be merging the two ID's into a single one which has both properties?

Ideally, yes... but...  The 31-bit mnt_id is currently exposed to userspace in
various places, e.g. /proc, sys_name_to_handle_at().  So we have to keep that
as is and we can't expand it.

For fsinfo(), however, it might make sense to only use the 64-bit uniquifier
as the identifier to use for direct look up.

However, looking up that identifier requires some sort of structure for doing
this and it's kind of worst case for the IDR tree as the keys are gradually
going to spread out, causing it to eat more memory.  It may be a tradeoff
worth making, and the memory consumption might not be that bad - or we could
use some other data structure such as an rbtree.

That's why I was going for the 31-bit identifier + uniquifier so that you can at
least tell if the identifier got recycled reasonably quickly.

David