Re: [PATCH v4 2/3] treewide: Switch memcpy() users of 'task->comm' to a more safer implementation

Dan Carpenter
2025-05-23T09:40:09Z

Hi Bhupesh,

kernel test robot noticed the following build warnings:

https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Bhupesh/exec-Remove-obsolete-comments/20250521-142443
base: https://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace for-next
patch link: https://lore.kernel.org/r/20250521062337.53262-3-bhupesh%40igalia.com
patch subject: [PATCH v4 2/3] treewide: Switch memcpy() users of 'task->comm' to a more safer implementation
config: powerpc64-randconfig-r071-20250522 (https://download.01.org/0day-ci/archive/20250522/202505221104.qV4Iy0rA-lkp@intel.com/config)
compiler: clang version 21.0.0git (https://github.com/llvm/llvm-project f819f46284f2a79790038e1f6649172789734ae8)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202505221104.qV4Iy0rA-lkp@intel.com/

New smatch warnings:
fs/coredump.c:591 do_coredump() error: buffer overflow 'comm' 16 <= 16
fs/coredump.c:1006 validate_coredump_safety() error: buffer overflow 'comm' 16 <= 16

vim +/comm +591 fs/coredump.c

a78282e2c94f4c Linus Torvalds 2024-09-26 524 void do_coredump(const kernel_siginfo_t *siginfo)
10c28d937e2cca Alex Kelly 2012-09-26 525 {
10c28d937e2cca Alex Kelly 2012-09-26 526 struct core_state core_state;
10c28d937e2cca Alex Kelly 2012-09-26 527 struct core_name cn;
10c28d937e2cca Alex Kelly 2012-09-26 528 struct mm_struct *mm = current->mm;
10c28d937e2cca Alex Kelly 2012-09-26 529 struct linux_binfmt * binfmt;
10c28d937e2cca Alex Kelly 2012-09-26 530 const struct cred *old_cred;
10c28d937e2cca Alex Kelly 2012-09-26 531 struct cred *cred;
a78282e2c94f4c Linus Torvalds 2024-09-26 532 int retval = 0;
10c28d937e2cca Alex Kelly 2012-09-26 533 int ispipe;
315c69261dd3fa Paul Wise 2019-08-02 534 size_t *argv = NULL;
315c69261dd3fa Paul Wise 2019-08-02 535 int argc = 0;
fbb1816942c044 Jann Horn 2015-09-09 536 /* require nonrelative corefile path and be extra careful */
fbb1816942c044 Jann Horn 2015-09-09 537 bool need_suid_safe = false;
acdedd99b0f3bf Oleg Nesterov 2013-04-30 538 bool core_dumped = false;
10c28d937e2cca Alex Kelly 2012-09-26 539 static atomic_t core_dump_count = ATOMIC_INIT(0);
10c28d937e2cca Alex Kelly 2012-09-26 540 struct coredump_params cprm = {
5ab1c309b34488 Denys Vlasenko 2012-10-04 541 .siginfo = siginfo,
10c28d937e2cca Alex Kelly 2012-09-26 542 .limit = rlimit(RLIMIT_CORE),
10c28d937e2cca Alex Kelly 2012-09-26 543 /*
10c28d937e2cca Alex Kelly 2012-09-26 544 * We must use the same mm->flags while dumping core to avoid
10c28d937e2cca Alex Kelly 2012-09-26 545 * inconsistency of bit flags, since this flag is not protected
10c28d937e2cca Alex Kelly 2012-09-26 546 * by any locks.
10c28d937e2cca Alex Kelly 2012-09-26 547 */
10c28d937e2cca Alex Kelly 2012-09-26 548 .mm_flags = mm->flags,
95c5436a488384 Eric W. Biederman 2022-03-08 549 .vma_meta = NULL,
8603b6f58637ce Oleksandr Natalenko 2022-09-03 550 .cpu = raw_smp_processor_id(),
10c28d937e2cca Alex Kelly 2012-09-26 551 };
10c28d937e2cca Alex Kelly 2012-09-26 552
5ab1c309b34488 Denys Vlasenko 2012-10-04 553 audit_core_dumps(siginfo->si_signo);
10c28d937e2cca Alex Kelly 2012-09-26 554
10c28d937e2cca Alex Kelly 2012-09-26 555 binfmt = mm->binfmt;
a78282e2c94f4c Linus Torvalds 2024-09-26 556 if (!binfmt || !binfmt->core_dump)
10c28d937e2cca Alex Kelly 2012-09-26 557 goto fail;
a78282e2c94f4c Linus Torvalds 2024-09-26 558 if (!__get_dumpable(cprm.mm_flags))
10c28d937e2cca Alex Kelly 2012-09-26 559 goto fail;
10c28d937e2cca Alex Kelly 2012-09-26 560
10c28d937e2cca Alex Kelly 2012-09-26 561 cred = prepare_creds();
a78282e2c94f4c Linus Torvalds 2024-09-26 562 if (!cred)
10c28d937e2cca Alex Kelly 2012-09-26 563 goto fail;
10c28d937e2cca Alex Kelly 2012-09-26 564 /*
10c28d937e2cca Alex Kelly 2012-09-26 565 * We cannot trust fsuid as being the "true" uid of the process
10c28d937e2cca Alex Kelly 2012-09-26 566 * nor do we know its entire history. We only know it was tainted
10c28d937e2cca Alex Kelly 2012-09-26 567 * so we dump it as root in mode 2, and only into a controlled
10c28d937e2cca Alex Kelly 2012-09-26 568 * environment (pipe handler or fully qualified path).
10c28d937e2cca Alex Kelly 2012-09-26 569 */
e579d2c259be42 Kees Cook 2013-02-27 570 if (__get_dumpable(cprm.mm_flags) == SUID_DUMP_ROOT) {
10c28d937e2cca Alex Kelly 2012-09-26 571 /* Setuid core dump mode */
10c28d937e2cca Alex Kelly 2012-09-26 572 cred->fsuid = GLOBAL_ROOT_UID; /* Dump root private */
fbb1816942c044 Jann Horn 2015-09-09 573 need_suid_safe = true;
10c28d937e2cca Alex Kelly 2012-09-26 574 }
10c28d937e2cca Alex Kelly 2012-09-26 575
5ab1c309b34488 Denys Vlasenko 2012-10-04 576 retval = coredump_wait(siginfo->si_signo, &core_state);
10c28d937e2cca Alex Kelly 2012-09-26 577 if (retval < 0)
10c28d937e2cca Alex Kelly 2012-09-26 578 goto fail_creds;
10c28d937e2cca Alex Kelly 2012-09-26 579
10c28d937e2cca Alex Kelly 2012-09-26 580 old_cred = override_creds(cred);
10c28d937e2cca Alex Kelly 2012-09-26 581
315c69261dd3fa Paul Wise 2019-08-02 582 ispipe = format_corename(&cn, &cprm, &argv, &argc);
10c28d937e2cca Alex Kelly 2012-09-26 583
10c28d937e2cca Alex Kelly 2012-09-26 584 if (ispipe) {
315c69261dd3fa Paul Wise 2019-08-02 585 int argi;
10c28d937e2cca Alex Kelly 2012-09-26 586 int dump_count;
10c28d937e2cca Alex Kelly 2012-09-26 587 char **helper_argv;
907ed1328d2a74 Lucas De Marchi 2013-04-30 588 struct subprocess_info *sub_info;
10c28d937e2cca Alex Kelly 2012-09-26 589
10c28d937e2cca Alex Kelly 2012-09-26 590 if (ispipe < 0) {
c114e9948c2b6a Roman Kisel 2024-07-18 @591 coredump_report_failure("format_corename failed, aborting core");
e7fd1549aeb83e Oleg Nesterov 2013-07-03 592 goto fail_unlock;

> /* This will always be NUL terminated. */ \
> - memcpy(comm, current->comm, sizeof(comm)); \
> + memcpy(comm, current->comm, TASK_COMM_LEN); \
> + comm[TASK_COMM_LEN] = '\0'; \
^^^^^^^^^^^^^^
This was supposed to be "TASK_COMM_LEN - 1". Also the comment says
it's not required...

10c28d937e2cca Alex Kelly 2012-09-26 593 }
10c28d937e2cca Alex Kelly 2012-09-26 594
10c28d937e2cca Alex Kelly 2012-09-26 595 if (cprm.limit == 1) {
10c28d937e2cca Alex Kelly 2012-09-26 596 /* See umh_pipe_setup() which sets RLIMIT_CORE = 1.

--
0-DAY CI Kernel Test Service