Re: Coverity: __do_sys_pidfd_send_signal(): UNINIT

[Kees Cook]

On Tue, Feb 13, 2024 at 05:18:01PM -0700, Tycho Andersen wrote:
> 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;

Ah yeah, coverity can't see through the hweight32 test. Sorry for the
noise!

-- 
Kees Cook