Re: [PATCH] liveupdate: truncate getfile name to NAME_MAX

[Pasha Tatashin <pasha.tatashin@soleen.com>]

On Tue, Mar 31, 2026 at 2:30 PM Luca Boccassi <luca.boccassi@gmail.com> wrote:
>
> On Tue, 31 Mar 2026, 19:21 Pasha Tatashin, <pasha.tatashin@soleen.com> wrote:
>>
>> On Tue, Mar 31, 2026 at 1:55 PM <luca.boccassi@gmail.com> wrote:
>> >
>> > From: Luca Boccassi <luca.boccassi@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?

>> >
>> > Signed-off-by: Luca Boccassi <luca.boccassi@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.

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?

Increasing this limit would require a kernel uAPI change, which
requires a very strong rationale, and would bump up the KHO ABI for
LUO. It also means that the LUO session data in:

struct luo_session_ser {
   char name[LIVEUPDATE_SESSION_NAME_LENGTH];
   struct luo_file_set_ser file_set_ser;
} __packed;

would get bigger, and thus the kernel would need to preserve more data
for every single session.

Ideally, this extra data should be preserved by the userspace
orchestrator via a separate memfd.

Pasha