[PATCH] liveupdate: truncate getfile name to NAME_MAX

[Luca Boccassi]

On Tue, 31 Mar 2026 at 20:14, Pasha Tatashin <pasha.tatashin at soleen.com> wrote:
>
> On Tue, Mar 31, 2026 at 2:30 PM Luca Boccassi <luca.boccassi at gmail.com> wrote:
> >
> > On Tue, 31 Mar 2026, 19:21 Pasha Tatashin, <pasha.tatashin at soleen.com> wrote:
> >>
> >> On Tue, Mar 31, 2026 at 1:55 PM <luca.boccassi at gmail.com> wrote:
> >> >
> >> > From: Luca Boccassi <luca.boccassi at gmail.com>
> >> >
> >> > dynamic_dname() harcodes its limit to NAME_MAX bytes, which includes a
> >> > NUL terminator and a 'anon_inode:' prefix. If the name passed on goes
> >> > above the limit it results in userspace getting a ENAMETOOLONG error.
> >> >
> >> > Truncate the name to NAME_MAX - 12 - 1 characters to ensure this never
> >> > fails (accounting for prefix and NUL).
> >> >
> >> > Fixes: 0153094d03df ("liveupdate: luo_session: add sessions support")
>
> Let's remove this, this patch is not fixing an existing issue, but a
> preparation for future uapi changes?

The linked patch that increases the buffer to 255 is tagged with
'fixes' so I don't mind, as that's enough to fix the -ENAMETOOLONG
error in currently released kernels. The original version of this set
the truncation to the current max buffer size, hence why I had the
fixes line.

> >> >
> >> > Signed-off-by: Luca Boccassi <luca.boccassi at gmail.com>
> >> > ---
> >> > Builds on top of this just sent by Aleksa:
> >> >
> >> > https://lore.kernel.org/linux-fsdevel/20260401-dynamic-dname-name_max-v1-1-8ca20ab2642e@amutable.com/
> >> >
> >> >  kernel/liveupdate/luo_session.c | 7 +++++--
> >> >  1 file changed, 5 insertions(+), 2 deletions(-)
> >> >
> >> > diff --git a/kernel/liveupdate/luo_session.c b/kernel/liveupdate/luo_session.c
> >> > index 7836772956406..4a40c7fdfb44f 100644
> >> > --- a/kernel/liveupdate/luo_session.c
> >> > +++ b/kernel/liveupdate/luo_session.c
> >> > @@ -366,11 +366,14 @@ static const struct file_operations luo_session_fops = {
> >> >  /* Create a "struct file" for session */
> >> >  static int luo_session_getfile(struct luo_session *session, struct file **filep)
> >> >  {
> >> > -       char name_buf[128];
> >> > +       char name_buf[NAME_MAX];
> >> >         struct file *file;
> >> >
> >> >         lockdep_assert_held(&session->mutex);
> >> > -       snprintf(name_buf, sizeof(name_buf), "[luo_session] %s", session->name);
> >> > +       /* dynamic_dname() rejects names above NAME_MAX bytes, including NUL terminator
> >> > +        * and a 'anon_inode:' prefix. Truncate to NAME_MAX - 12 - 1 to avoid
> >> > +        * ENAMETOOLONG. */
> >> > +       snprintf(name_buf, NAME_MAX - 12 - 1, "[luo_session] %s", session->name);
> >>
> >> I am confused by this change, the maximum session name length is: 64,
> >> as defined in:
> >> include/uapi/linux/liveupdate.h
> >> /* The maximum length of session name including null termination */
> >> #define LIVEUPDATE_SESSION_NAME_LENGTH 64
> >>
> >> So, 128 should be enough to include:
> >> "[luo_session] %s", session->name
> >>
> >> Or am I missing something?
> >>
> >> Pasha
> >
> >
> > Yeah we need to to bump that size, it's too small - see the shared doc. This is anyway needed for safety, as the user space call falls apart completely if it goes over the hard coded limit,
>
> If there is a safety issue, it should be fixed (even without a limit
> bump), could you please explain it.

Nothing specific, more from the abstract point of view - given that
this will fail with a hard error if it goes above the hardcoded limit,
it seems better to me to check it, rather than assuming it's going to
match, as that's error prone.

> I looked at the shared doc [https://tinyurl.com/luoddesign under limits tab]:
>
> I see:
> TODO: this needs to be bumped, as for the varlink API we will impose a
> cgroup path prefix, which means PATH_MAX + "/" + client-supplied
> string.
>
> Why would we do that? Why can't we use a 63-byte unique identifier for
> sessions and an in-memory database that maps the cgroup path prefix to
> session_ids, where the database also gets passed from one kernel to
> the other?

Because that requires tracking state across all runtime, and that's a
no-go. The session itself needs to be identifiable by cgroup, so that
its owner is uniquely identified in all cases, in all situations,
regardless of any state or lack thereof.
The structs have a size as input, so it should be doable to follow the
usual process and keep it backward compatible.