On Wed, Sep 13, 2023 at 05:22:34PM +0200, Miklos Szeredi wrote:
> If a mount is released then it's mnt_id can immediately be reused. This is
> bad news for user interfaces that want to uniquely identify a mount.
>
> Implementing a unique mount ID is trivial (use a 64bit counter).
> Unfortunately userspace assumes 32bit size and would overflow after the
> counter reaches 2^32.
>
> Introduce a new 64bit ID alongside the old one. Allow new interfaces to
> work on both the old and new IDs by starting the counter from 2^32.
>
> Signed-off-by: Miklos Szeredi <[email protected]>
> ---
> fs/mount.h | 3 ++-
> fs/namespace.c | 4 ++++
> fs/stat.c | 9 +++++++--
> include/uapi/linux/stat.h | 1 +
> 4 files changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/fs/mount.h b/fs/mount.h
> index 130c07c2f8d2..a14f762b3f29 100644
> --- a/fs/mount.h
> +++ b/fs/mount.h
> @@ -72,7 +72,8 @@ struct mount {
> struct fsnotify_mark_connector __rcu *mnt_fsnotify_marks;
> __u32 mnt_fsnotify_mask;
> #endif
> - int mnt_id; /* mount identifier */
> + int mnt_id; /* mount identifier, reused */
> + u64 mnt_id_unique; /* mount ID unique until reboot */
> int mnt_group_id; /* peer group identifier */
> int mnt_expiry_mark; /* true if marked for expiry */
> struct hlist_head mnt_pins;
> diff --git a/fs/namespace.c b/fs/namespace.c
> index e157efc54023..de47c5f66e17 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -68,6 +68,9 @@ static u64 event;
> static DEFINE_IDA(mnt_id_ida);
> static DEFINE_IDA(mnt_group_ida);
>
> +/* Don't allow confusion with mount ID allocated wit IDA */
> +static atomic64_t mnt_id_ctr = ATOMIC64_INIT(1ULL << 32);
Hm, is your concern that userspace confuses these two values? If so, I
think we shouldn't worry about this.
If a userspace program retrieves a mntid and then confuses itself about
what mnt id they're talking about something's very wrong anyway. So I'd
rather not see us waste 32 bits just for that. Other than that this
seems to implement what we agreed on at LSFMM so my hope is that this is
fairly uncontroversial.
On Thu, 14 Sept 2023 at 11:04, Christian Brauner <[email protected]> wrote:
>
> On Wed, Sep 13, 2023 at 05:22:34PM +0200, Miklos Szeredi wrote:
> > If a mount is released then it's mnt_id can immediately be reused. This is
> > bad news for user interfaces that want to uniquely identify a mount.
> >
> > Implementing a unique mount ID is trivial (use a 64bit counter).
> > Unfortunately userspace assumes 32bit size and would overflow after the
> > counter reaches 2^32.
> >
> > Introduce a new 64bit ID alongside the old one. Allow new interfaces to
> > work on both the old and new IDs by starting the counter from 2^32.
> >
> > Signed-off-by: Miklos Szeredi <[email protected]>
> > ---
> > fs/mount.h | 3 ++-
> > fs/namespace.c | 4 ++++
> > fs/stat.c | 9 +++++++--
> > include/uapi/linux/stat.h | 1 +
> > 4 files changed, 14 insertions(+), 3 deletions(-)
> >
> > diff --git a/fs/mount.h b/fs/mount.h
> > index 130c07c2f8d2..a14f762b3f29 100644
> > --- a/fs/mount.h
> > +++ b/fs/mount.h
> > @@ -72,7 +72,8 @@ struct mount {
> > struct fsnotify_mark_connector __rcu *mnt_fsnotify_marks;
> > __u32 mnt_fsnotify_mask;
> > #endif
> > - int mnt_id; /* mount identifier */
> > + int mnt_id; /* mount identifier, reused */
> > + u64 mnt_id_unique; /* mount ID unique until reboot */
> > int mnt_group_id; /* peer group identifier */
> > int mnt_expiry_mark; /* true if marked for expiry */
> > struct hlist_head mnt_pins;
> > diff --git a/fs/namespace.c b/fs/namespace.c
> > index e157efc54023..de47c5f66e17 100644
> > --- a/fs/namespace.c
> > +++ b/fs/namespace.c
> > @@ -68,6 +68,9 @@ static u64 event;
> > static DEFINE_IDA(mnt_id_ida);
> > static DEFINE_IDA(mnt_group_ida);
> >
> > +/* Don't allow confusion with mount ID allocated wit IDA */
> > +static atomic64_t mnt_id_ctr = ATOMIC64_INIT(1ULL << 32);
>
> Hm, is your concern that userspace confuses these two values? If so, I
> think we shouldn't worry about this.
Yes, one concern is that humans confuse the old and the new ID.
I also think it makes sense to allow the new interfaces to look up the
mount based on either the old or the new ID. But I could be wrong
there, since that might encourage bad code. Maybe the new interface
should only use take the new ID, which means no mixed use of
/proc/$$/mountinfo and statmnt/listmnt.
>
> If a userspace program retrieves a mntid and then confuses itself about
> what mnt id they're talking about something's very wrong anyway. So I'd
> rather not see us waste 32 bits just for that.
This is wasting a quarter of a billionth of the ID space. We are
surely not concerned about that.
Thanks,
Miklos