fix: F3: fix buffer overflow identified by security scanner

pkg/kando/process_client_signal.go

@@ -40,11 +40,11 @@ func runProcessClientSignal(cmd *cobra.Command, args []string) error {
 }
 
 func runProcessClientSignalWithOutput(out io.Writer, cmd *cobra.Command, args []string) error {
-	pid, err := strconv.Atoi(args[0])
+	pid, err := strconv.ParseInt(args[0], 0, 64)
 	if err != nil {
 		return err
 	}
-	signal, err := strconv.Atoi(args[1])
+	signal, err := strconv.ParseInt(args[1], 0, 64)
 	if err != nil {
 		return err
 	}
@@ -54,7 +54,7 @@ func runProcessClientSignalWithOutput(out io.Writer, cmd *cobra.Command, args []
 	}
 	asJSON := processAsJSONFlagValue(cmd)
 	cmd.SilenceUsage = true
-	p, err := kanx.SignalProcess(cmd.Context(), addr, int64(pid), int32(signal))
+	p, err := kanx.SignalProcess(cmd.Context(), addr, pid, signal)
 	if err != nil {
 		return err
 	}

pkg/kanx/client.go

@@ -80,7 +80,7 @@ func ListProcesses(ctx context.Context, addr string) ([]*Process, error) {
 	}
 }
 
-func SignalProcess(ctx context.Context, addr string, pid int64, signal int32) (*Process, error) {
+func SignalProcess(ctx context.Context, addr string, pid int64, signal int64) (*Process, error) {
 	conn, err := newGRPCConnection(addr)
 	if err != nil {
 		return nil, err

pkg/kanx/kanx.proto

@@ -27,7 +27,7 @@ message ProcessPidRequest {
 
 message SignalProcessRequest {
   int64 pid = 1;
-  int32 signal = 2;
+  int64 signal = 2;
 }
 
 message Process {

pkg/kanx/kanx_test.go

@@ -210,7 +210,7 @@ func (s *KanXSuite) TestSignalProcess_Int(c *C) {
 	c.Assert(err, IsNil)
 
 	// test SignalProcess, SIGINT
-	p0, err := SignalProcess(ctx, addr, p.GetPid(), int32(syscall.SIGINT))
+	p0, err := SignalProcess(ctx, addr, p.GetPid(), int64(syscall.SIGINT))
 	c.Assert(err, IsNil)
 	c.Assert(p0.GetPid(), Equals, p.GetPid())
 	c.Assert(p0.GetState(), Equals, ProcessState_PROCESS_STATE_RUNNING)
@@ -252,15 +252,15 @@ func (s *KanXSuite) TestSignalProcess_Stp(c *C) {
 	c.Assert(err, IsNil)
 
 	// test SignalProcess, SIGSTOP
-	p0, err := SignalProcess(ctx, addr, p.GetPid(), int32(syscall.SIGSTOP))
+	p0, err := SignalProcess(ctx, addr, p.GetPid(), int64(syscall.SIGSTOP))
 	c.Assert(err, IsNil)
 	c.Assert(p0.GetPid(), Equals, p.GetPid())
 	c.Assert(p0.GetState(), Equals, ProcessState_PROCESS_STATE_RUNNING)
 	c.Assert(p0.GetExitErr(), Equals, "")
 	c.Assert(p0.GetExitCode(), Equals, int64(0))
 
 	// test SignalProcess, SIGCONT
-	p0, err = SignalProcess(ctx, addr, p.GetPid(), int32(syscall.SIGCONT))
+	p0, err = SignalProcess(ctx, addr, p.GetPid(), int64(syscall.SIGCONT))
 	c.Assert(err, IsNil)
 	c.Assert(p0.GetPid(), Equals, p.GetPid())
 	c.Assert(p0.GetState(), Equals, ProcessState_PROCESS_STATE_RUNNING)
@@ -301,7 +301,7 @@ func (s *KanXSuite) TestSignalProcess_Kill(c *C) {
 	c.Assert(err, IsNil)
 
 	// test SignalProcess, SIGKILL
-	p0, err := SignalProcess(ctx, addr, p.GetPid(), int32(syscall.SIGKILL))
+	p0, err := SignalProcess(ctx, addr, p.GetPid(), int64(syscall.SIGKILL))
 	c.Assert(err, IsNil)
 	c.Assert(p0.GetPid(), Equals, p.GetPid())
 	c.Assert(p0.GetState(), Equals, ProcessState_PROCESS_STATE_RUNNING)

pkg/kanx/server.go

@@ -34,6 +34,8 @@ type processServiceServer struct {
 }
 
 type process struct {
+	// many reads on process data and only a write on process exit - use RWMutex.
+	// minimal risk of reads blocking writes.
 	mu       *sync.RWMutex
 	cmd      *exec.Cmd
 	doneCh   chan struct{}
@@ -90,14 +92,21 @@ func (s *processServiceServer) CreateProcess(_ context.Context, cpr *CreateProce
 	// one goroutine in server per forked process.  link between pid and output files will be lost
 	// if &process structure is lost.
 	go func() {
-		// wait until process is finished
+		// wait until process is finished.  do not use lock as there may be readers or writers
+		// on p and cmd is not expected to change (the state in cmd is system managed)
 		err := p.cmd.Wait()
 		// possible readers concurrent to write: lock the p structure for exit status update.
+		// keep the lock period as short as possible.  remove the possibility of blocking
+		// on log writes by moving them outside the region of the lock.
+		// go doesn't have lock promotion, so there's a small gap here from when Wait finishes
+		// until acquiring a write lock.
 		p.mu.Lock()
 		p.err = err
 		if exiterr, ok := err.(*exec.ExitError); ok {
 			p.exitCode = exiterr.ExitCode()
 		}
+		// no action will be taken on close errors, so just save the errors for logging
+		// later
 		stdoutErr := stdoutfd.Close()
 		stderrErr := stderrfd.Close()
 		can()
@@ -145,12 +154,14 @@ func (s *processServiceServer) SignalProcess(ctx context.Context, grp *SignalPro
 	}
 	// low level signal call
 	syssig := syscall.Signal(grp.Signal)
+	p.mu.Lock()
+	defer p.mu.Unlock()
 	err := p.cmd.Process.Signal(syssig)
 	if err != nil {
 		// `fault` tracks IPC errors
 		p.fault = err
 	}
-	return processToProtoWithLock(p), err
+	return processToProto(p), err
 }
 
 func (s *processServiceServer) ListProcesses(lpr *ListProcessesRequest, lps ProcessService_ListProcessesServer) error {