Re: [PATCH v2] nfs: check return value of various rpc calls

[Sascha Hauer <s.hauer@pengutronix.de>]

Hi Uwe,

I've rewritten this thing a little bit. First of all, this doesn't need
preprocessor tricks and also with this the nfserror to error mapping
function returns the error string, so we can convert printing the
messages to pr_* or dev_* functions. Also we use the human readable
error names for the errors we have a string for.

Sascha

-------------------------------8<--------------------------------------

From 3d764946914356eca94622a2eeeb4df459026d6d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= <u.kleine-koenig@pengutronix.de>
Date: Tue, 3 Nov 2020 21:09:32 +0100
Subject: [PATCH] nfs: check return value of various rpc calls
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Check more carefully for failing requests. This improves the error
message when trying to mount a non-exported nfs directory from:

	nfs_mount_req: file handle too big: 44831
to
	ERROR: NFS: Mounting failed: Permission denied

. This also fixes an out-of-bounds access as the filehandle size (44831
above) is read from just after the network packet in the error case.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 fs/nfs.c | 126 ++++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 115 insertions(+), 11 deletions(-)

diff --git a/fs/nfs.c b/fs/nfs.c
index 6c4637281d..db159f5ab8 100644
--- a/fs/nfs.c
+++ b/fs/nfs.c
@@ -263,6 +263,76 @@ struct nfs_dir {
 	struct nfs_fh fh;
 };
 
+struct nfserror {
+	int ne;
+	int e;
+	const char *name;
+};
+
+static struct nfserror nfserrors[] = {
+	{ .ne = NFS3ERR_PERM, .e = EPERM },
+	{ .ne = NFS3ERR_NOENT, .e = ENOENT },
+	{ .ne = NFS3ERR_IO, .e = EIO },
+	{ .ne = NFS3ERR_NXIO, .e = ENXIO },
+	{ .ne = NFS3ERR_ACCES, .e = EACCES },
+	{ .ne = NFS3ERR_EXIST, .e = EEXIST },
+	{ .ne = NFS3ERR_XDEV, .e = EXDEV },
+	{ .ne = NFS3ERR_NODEV, .e = ENODEV },
+	{ .ne = NFS3ERR_NOTDIR, .e = ENOTDIR },
+	{ .ne = NFS3ERR_ISDIR, .e = EISDIR },
+	{ .ne = NFS3ERR_INVAL, .e = EINVAL },
+	{ .ne = NFS3ERR_FBIG, .e = EFBIG },
+	{ .ne = NFS3ERR_NOSPC, .e = ENOSPC },
+	{ .ne = NFS3ERR_ROFS, .e = EROFS },
+	{ .ne = NFS3ERR_MLINK, .e = EMLINK },
+	{ .ne = NFS3ERR_NAMETOOLONG, .e = ENAMETOOLONG },
+	{ .ne = NFS3ERR_NOTEMPTY, .e = ENOTEMPTY },
+	{ .ne = NFS3ERR_DQUOT, .e = EDQUOT },
+	{ .ne = NFS3ERR_STALE, .e = ESTALE },
+	{ .ne = NFS3ERR_REMOTE, .e = EREMOTE },
+	{ .ne = NFS3ERR_NOTSUPP, .e = EOPNOTSUPP },
+	{ .ne = NFS3ERR_BADHANDLE, .e = EINVAL, .name = "BADHANDLE"},
+	{ .ne = NFS3ERR_NOT_SYNC, .e = EINVAL, .name = "NOT_SYNC" },
+	{ .ne = NFS3ERR_BAD_COOKIE, .e = EINVAL, .name = "BAD_COOKIE" },
+	{ .ne = NFS3ERR_TOOSMALL, .e = EINVAL, .name = "TOOSMALL" },
+	{ .ne = NFS3ERR_SERVERFAULT, .e = EINVAL, .name = "SERVERFAULT" },
+	{ .ne = NFS3ERR_BADTYPE, .e = EINVAL, .name = "BADTYPE" },
+	{ .ne = NFS3ERR_JUKEBOX, .e = EINVAL, .name = "JUKEBOX" },
+};
+
+static const char *nfserrstr(u32 nfserror, int *errcode)
+{
+#define BUFLEN 32
+	static char str[BUFLEN];
+	int i;
+
+	/*
+	 * Most NFS errors have a corresponding POSIX error code. But not all of
+	 * them have one, so some must be mapped to a different code here.
+	 */
+	for (i = 0; i < ARRAY_SIZE(nfserrors); i++) {
+		struct nfserror *err = &nfserrors[i];
+
+		if (nfserror == err->ne) {
+			if (errcode)
+				*errcode = -err->e;
+
+			if (err->name) {
+				snprintf(str, BUFLEN, "NFS3ERR_%s", err->name);
+				return str;
+			} else
+				return strerror(err->e);
+		}
+	}
+
+	if (errcode)
+		*errcode = -EINVAL;
+
+	snprintf(str, BUFLEN, "Unknown NFS error %d", nfserror);
+	return str;
+#undef BUFLEN
+}
+
 static void xdr_init(struct xdr_stream *stream, void *buf, int len)
 {
 	stream->p = stream->buf = buf;
@@ -642,7 +712,7 @@ static uint32_t *nfs_read_post_op_attr(uint32_t *p, struct inode *inode)
 static int nfs_mount_req(struct nfs_priv *npriv)
 {
 	uint32_t data[1024];
-	uint32_t *p;
+	uint32_t *p, status;
 	int len;
 	int pathlen;
 	struct packet *nfs_packet;
@@ -667,7 +737,18 @@ static int nfs_mount_req(struct nfs_priv *npriv)
 	if (IS_ERR(nfs_packet))
 		return PTR_ERR(nfs_packet);
 
-	p = (void *)nfs_packet->data + sizeof(struct rpc_reply) + 4;
+	p = (void *)nfs_packet->data + sizeof(struct rpc_reply);
+
+	/*
+	 * Theoretically the error status is one of MNT3ERR_..., but the NFS
+	 * constants are identical.
+	 */
+	status = ntoh32(net_read_uint32(p++));
+	if (status != NFS3_OK) {
+		int ret;
+		pr_err("Mounting failed: %s\n", nfserrstr(status, &ret));
+		return ret;
+	}
 
 	npriv->rootfh.size = ntoh32(net_read_uint32(p++));
 	if (npriv->rootfh.size > NFS3_FHSIZE) {
@@ -719,7 +800,7 @@ static int nfs_lookup_req(struct nfs_priv *npriv, struct nfs_fh *fh,
 {
 	struct nfs_inode *ninode = nfsi(inode);
 	uint32_t data[1024];
-	uint32_t *p;
+	uint32_t *p, status;
 	int len;
 	struct packet *nfs_packet;
 
@@ -761,7 +842,13 @@ static int nfs_lookup_req(struct nfs_priv *npriv, struct nfs_fh *fh,
 	if (IS_ERR(nfs_packet))
 		return PTR_ERR(nfs_packet);
 
-	p = (void *)nfs_packet->data + sizeof(struct rpc_reply) + 4;
+	p = (void *)nfs_packet->data + sizeof(struct rpc_reply);
+	status = ntoh32(net_read_uint32(p++));
+	if (status != NFS3_OK) {
+		int ret;
+		pr_err("Lookup failed: %s\n", nfserrstr(status, &ret));
+		return ret;
+	}
 
 	ninode->fh.size = ntoh32(net_read_uint32(p++));
 	if (ninode->fh.size > NFS3_FHSIZE) {
@@ -787,7 +874,7 @@ static int nfs_lookup_req(struct nfs_priv *npriv, struct nfs_fh *fh,
 static void *nfs_readdirattr_req(struct nfs_priv *npriv, struct nfs_dir *dir)
 {
 	uint32_t data[1024];
-	uint32_t *p;
+	uint32_t *p, status;
 	int len;
 	struct packet *nfs_packet;
 	void *buf;
@@ -845,7 +932,13 @@ static void *nfs_readdirattr_req(struct nfs_priv *npriv, struct nfs_dir *dir)
 	if (IS_ERR(nfs_packet))
 		return NULL;
 
-	p = (void *)nfs_packet->data + sizeof(struct rpc_reply) + 4;
+	p = (void *)nfs_packet->data + sizeof(struct rpc_reply);
+	status = ntoh32(net_read_uint32(p++));
+	if (status != NFS3_OK) {
+		pr_err("Readdir failed: %s\n", nfserrstr(status, NULL));
+		return NULL;
+	}
+
 	p = nfs_read_post_op_attr(p, NULL);
 
 	/* update cookieverf */
@@ -879,8 +972,8 @@ static int nfs_read_req(struct file_priv *priv, uint64_t offset,
 		uint32_t readlen)
 {
 	uint32_t data[1024];
-	uint32_t *p;
-	int len;
+	uint32_t *p, status;
+	int len, ret;
 	struct packet *nfs_packet;
 	uint32_t rlen, eof;
 
@@ -922,7 +1015,12 @@ static int nfs_read_req(struct file_priv *priv, uint64_t offset,
 	if (IS_ERR(nfs_packet))
 		return PTR_ERR(nfs_packet);
 
-	p = (void *)nfs_packet->data + sizeof(struct rpc_reply) + 4;
+	p = (void *)nfs_packet->data + sizeof(struct rpc_reply);
+	status = ntoh32(net_read_uint32(p++));
+	if (status != NFS3_OK) {
+		pr_err("Read failed: %s\n", nfserrstr(status, &ret));
+		return ret;
+	}
 
 	p = nfs_read_post_op_attr(p, NULL);
 
@@ -981,7 +1079,7 @@ static int nfs_readlink_req(struct nfs_priv *npriv, struct nfs_fh *fh,
 			    char **target)
 {
 	uint32_t data[1024];
-	uint32_t *p;
+	uint32_t *p, status;
 	uint32_t len;
 	struct packet *nfs_packet;
 
@@ -1017,7 +1115,13 @@ static int nfs_readlink_req(struct nfs_priv *npriv, struct nfs_fh *fh,
 	if (IS_ERR(nfs_packet))
 		return PTR_ERR(nfs_packet);
 
-	p = (void *)nfs_packet->data + sizeof(struct rpc_reply) + 4;
+	p = (void *)nfs_packet->data + sizeof(struct rpc_reply);
+	status = ntoh32(net_read_uint32(p++));
+	if (status != NFS3_OK) {
+		int ret;
+		pr_err("Readlink failed: %s\n", nfserrstr(status, &ret));
+		return ret;
+	}
 
 	p = nfs_read_post_op_attr(p, NULL);
 
-- 
2.20.1

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox