On Tue, Feb 13, 2024 at 03:59:37PM -0800, coverity-bot wrote: > Hello! > > This is an experimental semi-automated report about issues detected by > Coverity from a scan of next-20240213 as part of the linux-next scan project: > https://scan.coverity.com/projects/linux-next-weekly-scan > > You're getting this email because you were associated with the identified > lines of code (noted below) that were touched by commits: > > Sat Feb 10 22:37:25 2024 +0100 > 3f643cd23510 ("pidfd: allow to override signal scope in pidfd_send_signal()") > Sat Feb 10 22:37:23 2024 +0100 > 81b9d8ac0640 ("pidfd: change pidfd_send_signal() to respect PIDFD_THREAD") > > Coverity reported the following: > > *** CID 1583637: (UNINIT) > kernel/signal.c:3963 in __do_sys_pidfd_send_signal() > 3957 /* Only allow sending arbitrary signals to yourself. */ > 3958 ret = -EPERM; > 3959 if ((task_pid(current) != pid) && > 3960 (kinfo.si_code >= 0 || kinfo.si_code == SI_TKILL)) > 3961 goto err; > 3962 } else { > vvv CID 1583637: (UNINIT) > vvv Using uninitialized value "type" when calling "prepare_kill_siginfo". > 3963 prepare_kill_siginfo(sig, &kinfo, type); > 3964 } > 3965 > 3966 if (type == PIDTYPE_PGID) > 3967 ret = kill_pgrp_info(sig, &kinfo, pid); > 3968 else > kernel/signal.c:3966 in __do_sys_pidfd_send_signal() > 3960 (kinfo.si_code >= 0 || kinfo.si_code == SI_TKILL)) > 3961 goto err; > 3962 } else { > 3963 prepare_kill_siginfo(sig, &kinfo, type); > 3964 } > 3965 > vvv CID 1583637: (UNINIT) > vvv Using uninitialized value "type". > 3966 if (type == PIDTYPE_PGID) > 3967 ret = kill_pgrp_info(sig, &kinfo, pid); > 3968 else > 3969 ret = kill_pid_info_type(sig, &kinfo, pid, type); > 3970 err: > 3971 fdput(f); > > If this is a false positive, please let us know so we can mark it as > such, or teach the Coverity rules to be smarter. If not, please make > sure fixes get into linux-next. :) For patches fixing this, please > include these lines (but double-check the "Fixes" first): I think this is a false positive, we have: /* Enforce flags be set to 0 until we add an extension. */ if (flags & ~PIDFD_SEND_SIGNAL_FLAGS) return -EINVAL; /* Ensure that only a single signal scope determining flag is set. */ if (hweight32(flags & PIDFD_SEND_SIGNAL_FLAGS) > 1) return -EINVAL; which should enforce that at most one bit is set, and there's a case statement for each of these bits in the later switch, switch (flags) { case 0: /* Infer scope from the type of pidfd. */ if (f.file->f_flags & PIDFD_THREAD) type = PIDTYPE_PID; else type = PIDTYPE_TGID; break; case PIDFD_SIGNAL_THREAD: type = PIDTYPE_PID; break; case PIDFD_SIGNAL_THREAD_GROUP: type = PIDTYPE_TGID; break; case PIDFD_SIGNAL_PROCESS_GROUP: type = PIDTYPE_PGID; break; } That said, a default case wouldn't hurt, and we should fix the first comment anyways, since now we have extensions. I'm happy to send a patch or maybe it's better for Christian to fix it in-tree. Tycho
