From f3ff82093eb55b5b53ad99576f8181867ecfedd0 Mon Sep 17 00:00:00 2001
From: Ayush Ranjan <ayushranjan@google.com>
Date: Mon, 24 Jan 2022 21:58:50 -0800
Subject: [PATCH] Wrap server-side panics in EREMOTEIO.

EREMOTEIO is a more appropriate generic error for a remote procedure call (RPC)
failure on the gofer. EFAULT means bad address and can be misleading to the
application as it will denote a MM layer related issue.

PiperOrigin-RevId: 423990374
---
 pkg/lisafs/connection.go     | 19 +++++++++++++++++--
 pkg/p9/p9test/client_test.go |  4 ++--
 pkg/p9/server.go             |  4 ++--
 3 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/pkg/lisafs/connection.go b/pkg/lisafs/connection.go
index 861a0ab9a..0435dbbb0 100644
--- a/pkg/lisafs/connection.go
+++ b/pkg/lisafs/connection.go
@@ -15,6 +15,8 @@
 package lisafs
 
 import (
+	"runtime/debug"
+
 	"golang.org/x/sys/unix"
 	"gvisor.dev/gvisor/pkg/flipcall"
 	"gvisor.dev/gvisor/pkg/log"
@@ -156,7 +158,7 @@ func (c *Connection) respondError(comm Communicator, err unix.Errno) (MID, uint3
 	return Error, respLen, nil
 }
 
-func (c *Connection) handleMsg(comm Communicator, m MID, payloadLen uint32) (MID, uint32, []int) {
+func (c *Connection) handleMsg(comm Communicator, m MID, payloadLen uint32) (retM MID, retPayloadLen uint32, retFDs []int) {
 	if payloadLen > c.maxMessageSize {
 		log.Warningf("received payload is too large: %d bytes", payloadLen)
 		return c.respondError(comm, unix.EIO)
@@ -165,7 +167,20 @@ func (c *Connection) handleMsg(comm Communicator, m MID, payloadLen uint32) (MID
 		// c.close() has been called; the connection is shutting down.
 		return c.respondError(comm, unix.ECONNRESET)
 	}
-	defer c.reqGate.Leave()
+	defer func() {
+		c.reqGate.Leave()
+
+		// Don't allow a panic to propagate.
+		if err := recover(); err != nil {
+			// Include a useful log message.
+			log.Warningf("panic in handler: %v\n%s", err, debug.Stack())
+
+			// Wrap in an EREMOTEIO error; we don't really have a better way to
+			// describe this kind of error. EREMOTEIO is appropriate for a generic
+			// failed RPC message.
+			retM, retPayloadLen, retFDs = c.respondError(comm, unix.EREMOTEIO)
+		}
+	}()
 
 	if !c.mounted && m != Mount {
 		log.Warningf("connection must first be mounted")
diff --git a/pkg/p9/p9test/client_test.go b/pkg/p9/p9test/client_test.go
index bb77e8e5f..e6be333ec 100644
--- a/pkg/p9/p9test/client_test.go
+++ b/pkg/p9/p9test/client_test.go
@@ -45,8 +45,8 @@ func TestPanic(t *testing.T) {
 	})
 
 	// Attach to the client.
-	if _, err := c.Attach("/"); err != unix.EFAULT {
-		t.Fatalf("got attach err %v, want EFAULT", err)
+	if _, err := c.Attach("/"); err != unix.EREMOTEIO {
+		t.Fatalf("got attach err %v, want EREMOTEIO", err)
 	}
 }
 
diff --git a/pkg/p9/server.go b/pkg/p9/server.go
index 4866743cc..fc6d96828 100644
--- a/pkg/p9/server.go
+++ b/pkg/p9/server.go
@@ -494,10 +494,10 @@ func (cs *connState) handle(m message) (r message) {
 			// Include a useful log message.
 			log.Warningf("panic in handler: %v\n%s", err, debug.Stack())
 
-			// Wrap in an EFAULT error; we don't really have a
+			// Wrap in an EREMOTEIO error; we don't really have a
 			// better way to describe this kind of error. It will
 			// usually manifest as a result of the test framework.
-			r = newErrFromLinuxerr(linuxerr.EFAULT)
+			r = newErrFromLinuxerr(linuxerr.EREMOTEIO)
 		}
 	}()
 	if handler, ok := m.(handler); ok {