Tycho Andersen <tycho@tycho.pizza> writes:
> On Wed, Sep 25, 2024 at 09:09:18PM -0500, Eric W. Biederman wrote:
>> Tycho Andersen <tycho@tycho.pizza> writes:
>>
>> > Yep, I did this for the test above, and it worked fine:
>> >
>> > if (bprm->fdpath) {
>> > /*
>> > * If fdpath was set, execveat() made up a path that will
>> > * probably not be useful to admins running ps or similar.
>> > * Let's fix it up to be something reasonable.
>> > */
>> > struct path root;
>> > char *path, buf[1024];
>> >
>> > get_fs_root(current->fs, &root);
>> > path = __d_path(&bprm->file->f_path, &root, buf, sizeof(buf));
>> >
>> > __set_task_comm(me, kbasename(path), true);
>> > } else {
>> > __set_task_comm(me, kbasename(bprm->filename), true);
>> > }
>> >
>> > obviously we don't want a stack allocated buffer, but triggering on
>> > ->fdpath != NULL seems like the right thing, so we won't need a flag
>> > either.
>> >
>> > The question is: argv[0] or __d_path()?
>>
>> You know. I think we can just do:
>>
>> BUILD_BUG_ON(DNAME_INLINE_LEN >= TASK_COMM_LEN);
>> __set_task_comm(me, bprm->file->f_path.dentry->d_name.name, true);
>>
>> Barring cache misses that should be faster and more reliable than what
>> we currently have and produce the same output in all of the cases we
>> like, and produce better output in all of the cases that are a problem
>> today.
>>
>> Does anyone see any problem with that?
>
> Nice, this works great. We need to drop the BUILD_BUG_ON() since it is
> violated in today's tree, but I think this is safe to do anyway since
> __set_task_comm() does strscpy_pad(tsk->comm, buf, sizeof(tsk->comm)).
Doh. I simply put the conditional in the wrong order. That should have
been:
BUILD_BUG_ON(TASK_COMM_LEN > DNAME_INLINE_LEN);
Sorry I was thinking of the invariant that needs to be preserved rather
than the bug that happens.
Eric
