Justin Stitt <[email protected]> writes: > strncpy() is deprecated for use on NUL-terminated destination strings > [1] and as such we should prefer more robust and less ambiguous string > interfaces. > > In every other location psinfo->pr_fname is used, it's with strscpy_pad. > It's clear that this field needs to be NUL-terminated and potentially > NUL-padded as well. > binfmt_elf.c +1545: > | char *__get_task_comm(char *buf, size_t buf_size, struct task_struct > *tsk) > | { > | task_lock(tsk); > | /* Always NUL terminated and zero-padded */ > | strscpy_pad(buf, tsk->comm, buf_size); > | task_unlock(tsk); > | return buf; > | } > > Note that this patch relies on the _new_ 2-argument versions of > strscpy() and strscpy_pad() introduced in Commit e6584c3964f2f ("string: > Allow 2-argument strscpy()").
I am perplexed. Why not use get_task_comm fill_psinfo like binfmt_elf does? It seems very silly to copy half the function without locking and then not copy it's locking as well. Given that the more highly tested binfmt_elf uses get_task_comm I can't imagine a reason why binfmt_elf_fdpic can't use it as well. Eric > Link: > https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings > [1] > Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html > [2] > Link: https://github.com/KSPP/linux/issues/90 > Cc: [email protected] > Signed-off-by: Justin Stitt <[email protected]> > --- > Note: build-tested only. > > Found with: $ rg "strncpy\(" > --- > fs/binfmt_elf_fdpic.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c > index 1920ed69279b..0365f14f18fc 100644 > --- a/fs/binfmt_elf_fdpic.c > +++ b/fs/binfmt_elf_fdpic.c > @@ -1359,7 +1359,7 @@ static int fill_psinfo(struct elf_prpsinfo *psinfo, > struct task_struct *p, > SET_UID(psinfo->pr_uid, from_kuid_munged(cred->user_ns, cred->uid)); > SET_GID(psinfo->pr_gid, from_kgid_munged(cred->user_ns, cred->gid)); > rcu_read_unlock(); > - strncpy(psinfo->pr_fname, p->comm, sizeof(psinfo->pr_fname)); > + strscpy_pad(psinfo->pr_fname, p->comm); > > return 0; > } > > --- > base-commit: a4145ce1e7bc247fd6f2846e8699473448717b37 > change-id: 20240320-strncpy-fs-binfmt_elf_fdpic-c-828286d76310 > > Best regards, > -- > Justin Stitt <[email protected]>
