* [PATCH 0/4] fs: nfs: Fix buffer overflows
@ 2025-11-04  8:08 Sascha Hauer
  2025-11-04  8:08 ` [PATCH 1/4] fs: nfs: drop PROG_NFS special casing Sascha Hauer
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Sascha Hauer @ 2025-11-04  8:08 UTC (permalink / raw)
  To: BAREBOX
The NFS does hardly any checks on incoming packets. It blindly trusts
the length values in packets and happily reads past the packets. This
series introduces a function to read from packets concentrating the
boundary checks in a single place. Also some other things I stumbled
upon along the way.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
Sascha Hauer (4):
      fs: nfs: drop PROG_NFS special casing
      fs: nfs: do not read past packets
      fs: nfs: use dev_* for messages
      fs: nfs: reduce unwanted message to debug level
 fs/nfs.c | 379 ++++++++++++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 267 insertions(+), 112 deletions(-)
---
base-commit: 7605338da017da2492d83e919f7a88eefe4cd306
change-id: 20251104-nfs-0af879139539
Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>
^ permalink raw reply	[flat|nested] 5+ messages in thread
* [PATCH 1/4] fs: nfs: drop PROG_NFS special casing
  2025-11-04  8:08 [PATCH 0/4] fs: nfs: Fix buffer overflows Sascha Hauer
@ 2025-11-04  8:08 ` Sascha Hauer
  2025-11-04  8:08 ` [PATCH 2/4] fs: nfs: do not read past packets Sascha Hauer
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Sascha Hauer @ 2025-11-04  8:08 UTC (permalink / raw)
  To: BAREBOX
For rpc_prog == PROG_NFS the nfs error is checked in rpc_check_reply()
and erroneous packets are discarded right away. This is unnecessary
since the nfs error is checked by the callers anyway. Drop the duplicate
error checking to simplify the code.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 fs/nfs.c | 26 +++-----------------------
 1 file changed, 3 insertions(+), 23 deletions(-)
diff --git a/fs/nfs.c b/fs/nfs.c
index 9c72d709fb44fa1f4fb60f7aa93bd1733fa50efa..ef1ab1a6e6497fb40553dde16793f2e1a8808b53 100644
--- a/fs/nfs.c
+++ b/fs/nfs.c
@@ -421,14 +421,10 @@ static uint32_t *rpc_add_credentials(uint32_t *p)
 	return p;
 }
 
-static int rpc_check_reply(struct packet *pkt, int rpc_prog,
-			   uint32_t rpc_id, int *nfserr)
+static int rpc_check_reply(struct packet *pkt, uint32_t rpc_id)
 {
-	uint32_t *data;
 	struct rpc_reply rpc;
 
-	*nfserr = 0;
-
 	memcpy(&rpc, pkt->data, sizeof(rpc));
 
 	if (ntoh32(rpc.id) != rpc_id)
@@ -440,15 +436,6 @@ static int rpc_check_reply(struct packet *pkt, int rpc_prog,
 		return -EINVAL;
 	}
 
-	if (rpc_prog != PROG_NFS)
-		return 0;
-
-	data = (uint32_t *)(pkt->data + sizeof(struct rpc_reply));
-	*nfserr = ntoh32(net_read_uint32(data));
-	*nfserr = -*nfserr;
-
-	debug("%s: err %d\n", __func__, *nfserr);
-
 	return 0;
 }
 
@@ -468,7 +455,6 @@ static struct packet *rpc_req(struct nfs_priv *npriv, int rpc_prog,
 	unsigned short dport;
 	int ret;
 	unsigned char *payload = net_udp_get_payload(npriv->con);
-	int nfserr;
 	int tries = 0;
 	struct packet *packet;
 
@@ -530,8 +516,7 @@ static struct packet *rpc_req(struct nfs_priv *npriv, int rpc_prog,
 
 		packet = list_first_entry(&npriv->packets, struct packet, list);
 
-		ret = rpc_check_reply(packet, rpc_prog,
-				      npriv->rpc_id, &nfserr);
+		ret = rpc_check_reply(packet, npriv->rpc_id);
 		if (ret == -EAGAIN) {
 			nfs_free_packet(packet);
 			continue;
@@ -539,12 +524,7 @@ static struct packet *rpc_req(struct nfs_priv *npriv, int rpc_prog,
 			nfs_free_packet(packet);
 			return ERR_PTR(ret);
 		} else {
-			if (rpc_prog == PROG_NFS && nfserr) {
-				nfs_free_packet(packet);
-				return ERR_PTR(nfserr);
-			} else {
-				return packet;
-			}
+			return packet;
 		}
 	}
 }
-- 
2.47.3
^ permalink raw reply	[flat|nested] 5+ messages in thread
* [PATCH 2/4] fs: nfs: do not read past packets
  2025-11-04  8:08 [PATCH 0/4] fs: nfs: Fix buffer overflows Sascha Hauer
  2025-11-04  8:08 ` [PATCH 1/4] fs: nfs: drop PROG_NFS special casing Sascha Hauer
@ 2025-11-04  8:08 ` Sascha Hauer
  2025-11-04  8:08 ` [PATCH 3/4] fs: nfs: use dev_* for messages Sascha Hauer
  2025-11-04  8:08 ` [PATCH 4/4] fs: nfs: reduce unwanted message to debug level Sascha Hauer
  3 siblings, 0 replies; 5+ messages in thread
From: Sascha Hauer @ 2025-11-04  8:08 UTC (permalink / raw)
  To: BAREBOX
The NFS code has hardly any length checks and it blindly trusts length
values in incoming packets. Fix this by introducing a nfs_packet_read()
which is consistently used to read from an incoming packet.
After a packet has been consumed it must be freed with
nfs_free_packet(). This wasn't done in several error cases and is fixed
here as well.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 fs/nfs.c | 288 +++++++++++++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 224 insertions(+), 64 deletions(-)
diff --git a/fs/nfs.c b/fs/nfs.c
index ef1ab1a6e6497fb40553dde16793f2e1a8808b53..163fd2182c0fc63d88725f0bba6ad5d1a71e2309 100644
--- a/fs/nfs.c
+++ b/fs/nfs.c
@@ -130,7 +130,8 @@ struct nfs_fh {
 
 struct packet {
 	struct list_head list;
-	int len;
+	unsigned int len;
+	unsigned int pos;
 	char data[];
 };
 
@@ -393,6 +394,51 @@ static int decode_filename(struct xdr_stream *xdr, char *name, u32 *length)
 	return -EIO;
 }
 
+/*
+ * Read @len bytes from a NFS packets. Returns a pointer to the data
+ * or NULL if the packet doesn't contain enough data
+ */
+static void *nfs_packet_read(struct packet *nfs_packet, unsigned int len)
+{
+	void *ret;
+
+	if (nfs_packet->pos + len > nfs_packet->len) {
+		nfs_packet->pos = nfs_packet->len;
+		return NULL;
+	}
+
+	ret = &nfs_packet->data[0];
+	ret += nfs_packet->pos;
+
+	nfs_packet->pos += len;
+
+	return ret;
+}
+
+/*
+ * Return the remaining packet data as an allocated buffer
+ */
+static void *nfs_packet_memdup_rest(struct packet *nfs_packet, unsigned int *len)
+{
+	void *p;
+
+	*len = nfs_packet->len - nfs_packet->pos;
+
+	p = nfs_packet_read(nfs_packet, *len);
+	if (!p)
+		return NULL;
+
+	return xmemdup(p, *len);
+}
+
+/*
+ * Align the read pointer of a NFS packet
+ */
+static void nfs_read_align(struct packet *nfs_packet, unsigned int align)
+{
+	nfs_packet->pos = ALIGN(nfs_packet->pos, align);
+}
+
 /*
  * rpc_add_credentials - Add RPC authentication/verifier entries
  */
@@ -424,8 +470,13 @@ static uint32_t *rpc_add_credentials(uint32_t *p)
 static int rpc_check_reply(struct packet *pkt, uint32_t rpc_id)
 {
 	struct rpc_reply rpc;
+	void *p;
+
+	p = nfs_packet_read(pkt, sizeof(rpc));
+	if (!p)
+		return -EINVAL;
 
-	memcpy(&rpc, pkt->data, sizeof(rpc));
+	memcpy(&rpc, p, sizeof(rpc));
 
 	if (ntoh32(rpc.id) != rpc_id)
 		return -EAGAIN;
@@ -537,6 +588,8 @@ static int rpc_lookup_req(struct nfs_priv *npriv, uint32_t prog, uint32_t ver)
 	uint32_t data[16];
 	struct packet *nfs_packet;
 	uint32_t port;
+	void *p;
+	int ret;
 
 	data[0] = 0; data[1] = 0;	/* auth credential */
 	data[2] = 0; data[3] = 0;	/* auth verifier */
@@ -549,7 +602,13 @@ static int rpc_lookup_req(struct nfs_priv *npriv, uint32_t prog, uint32_t ver)
 	if (IS_ERR(nfs_packet))
 		return PTR_ERR(nfs_packet);
 
-	port = ntoh32(net_read_uint32(nfs_packet->data + sizeof(struct rpc_reply)));
+	p = nfs_packet_read(nfs_packet, sizeof(uint32_t));
+	if (!p) {
+		ret = -EINVAL;
+		goto err_free_packet;
+	}
+
+	port = ntoh32(net_read_uint32(p));
 
 	nfs_free_packet(nfs_packet);
 
@@ -560,6 +619,11 @@ static int rpc_lookup_req(struct nfs_priv *npriv, uint32_t prog, uint32_t ver)
 	}
 
 	return port;
+
+err_free_packet:
+	nfs_free_packet(nfs_packet);
+
+	return ret;
 }
 
 static uint32_t *nfs_add_uint32(uint32_t *p, uint32_t val)
@@ -672,8 +736,10 @@ static int nfs_fattr3_to_stat(uint32_t *p, struct inode *inode)
 	return 0;
 }
 
-static uint32_t *nfs_read_post_op_attr(uint32_t *p, struct inode *inode)
+static int nfs_read_post_op_attr(struct packet *nfs_packet, struct inode *inode)
 {
+	void *p;
+
 	/*
 	 * union post_op_attr switch (bool attributes_follow) {
 	 * case TRUE:
@@ -683,12 +749,18 @@ static uint32_t *nfs_read_post_op_attr(uint32_t *p, struct inode *inode)
 	 * };
 	 */
 
-	if (ntoh32(net_read_uint32(p++))) {
+	p = nfs_packet_read(nfs_packet, sizeof(uint32_t));
+	if (!p)
+		return -EINVAL;
+
+	if (ntoh32(net_read_uint32(p))) {
+		p = nfs_packet_read(nfs_packet, 21 * sizeof(uint32_t));
+		if (!p)
+			return -EINVAL;
 		nfs_fattr3_to_stat(p, inode);
-		p += 21;
 	}
 
-	return p;
+	return 0;
 }
 
 /*
@@ -700,6 +772,7 @@ static int nfs_mount_req(struct nfs_priv *npriv)
 	uint32_t *p, status;
 	int len;
 	int pathlen;
+	int ret;
 	struct packet *nfs_packet;
 
 	pathlen = strlen(npriv->path);
@@ -722,31 +795,50 @@ 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);
-
 	/*
 	 * Theoretically the error status is one of MNT3ERR_..., but the NFS
 	 * constants are identical.
 	 */
-	status = ntoh32(net_read_uint32(p++));
+	p = nfs_packet_read(nfs_packet, sizeof(uint32_t));
+	if (!p) {
+		ret = -EINVAL;
+		goto err_free_packet;
+	}
+
+	status = ntoh32(net_read_uint32(p));
 	if (status != NFS3_OK) {
-		int ret;
 		pr_err("Mounting failed: %s\n", nfserrstr(status, &ret));
-		return ret;
+		goto err_free_packet;
 	}
 
-	npriv->rootfh.size = ntoh32(net_read_uint32(p++));
+	p = nfs_packet_read(nfs_packet, sizeof(uint32_t));
+	if (!p) {
+		ret = -EINVAL;
+		goto err_free_packet;
+	}
+
+	npriv->rootfh.size = ntoh32(net_read_uint32(p));
 	if (npriv->rootfh.size > NFS3_FHSIZE) {
 		printf("%s: file handle too big: %lu\n",
 		       __func__, (unsigned long)npriv->rootfh.size);
-		nfs_free_packet(nfs_packet);
-		return -EIO;
+		ret = -EIO;
+		goto err_free_packet;
+	}
+
+	p = nfs_packet_read(nfs_packet, npriv->rootfh.size);
+	if (!p) {
+		ret = -EINVAL;
+		goto err_free_packet;
 	}
+
 	memcpy(npriv->rootfh.data, p, npriv->rootfh.size);
 
+	ret = 0;
+
+err_free_packet:
 	nfs_free_packet(nfs_packet);
 
-	return 0;
+	return ret;
 }
 
 /*
@@ -787,6 +879,7 @@ static int nfs_lookup_req(struct nfs_priv *npriv, struct nfs_fh *fh,
 	uint32_t data[1024];
 	uint32_t *p, status;
 	int len;
+	int ret = 0;
 	struct packet *nfs_packet;
 
 	/*
@@ -827,29 +920,50 @@ 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);
-	status = ntoh32(net_read_uint32(p++));
+	p = nfs_packet_read(nfs_packet, sizeof(uint32_t));
+	if (!p) {
+		ret = -EINVAL;
+		goto err_free_packet;
+	}
+
+	status = ntoh32(net_read_uint32(p));
 	if (status != NFS3_OK) {
-		int ret;
 		pr_err("Lookup failed: %s\n", nfserrstr(status, &ret));
-		return ret;
+		goto err_free_packet;
 	}
 
-	ninode->fh.size = ntoh32(net_read_uint32(p++));
+	p = nfs_packet_read(nfs_packet, sizeof(uint32_t));
+	if (!p) {
+		ret = -EINVAL;
+		goto err_free_packet;
+	}
+
+	ninode->fh.size = ntoh32(net_read_uint32(p));
 	if (ninode->fh.size > NFS3_FHSIZE) {
-		nfs_free_packet(nfs_packet);
 		debug("%s: file handle too big: %u\n", __func__,
 		      ninode->fh.size);
-		return -EIO;
+		ret = -EIO;
+		goto err_free_packet;
 	}
+
+	p = nfs_packet_read(nfs_packet, ninode->fh.size);
+	if (!p) {
+		ret = -EINVAL;
+		goto err_free_packet;
+	}
+
 	memcpy(ninode->fh.data, p, ninode->fh.size);
-	p += DIV_ROUND_UP(ninode->fh.size, 4);
 
-	nfs_read_post_op_attr(p, inode);
+	nfs_read_align(nfs_packet, 4);
 
+	nfs_read_post_op_attr(nfs_packet, inode);
+
+	ret = 0;
+
+err_free_packet:
 	nfs_free_packet(nfs_packet);
 
-	return 0;
+	return ret;
 }
 
 /*
@@ -862,7 +976,8 @@ static void *nfs_readdirattr_req(struct nfs_priv *npriv, struct nfs_dir *dir)
 	uint32_t *p, status;
 	int len;
 	struct packet *nfs_packet;
-	void *buf;
+	void *buf = NULL;
+	int ret;
 
 	/*
 	 * struct READDIR3args {
@@ -917,37 +1032,43 @@ 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);
-	status = ntoh32(net_read_uint32(p++));
+	p = nfs_packet_read(nfs_packet, sizeof(uint32_t));
+	if (!p) {
+		ret = -EINVAL;
+		goto err_free_packet;
+	}
+
+	status = ntoh32(net_read_uint32(p));
 	if (status != NFS3_OK) {
 		pr_err("Readdir failed: %s\n", nfserrstr(status, NULL));
-		return NULL;
+		ret = -EIO;
+		goto err_free_packet;
 	}
 
-	p = nfs_read_post_op_attr(p, NULL);
+	ret = nfs_read_post_op_attr(nfs_packet, NULL);
+	if (ret)
+		goto err_free_packet;
 
 	/* update cookieverf */
-	memcpy(dir->cookieverf, p, NFS3_COOKIEVERFSIZE);
-	p += NFS3_COOKIEVERFSIZE / 4;
-
-	len = (void *)nfs_packet->data + nfs_packet->len - (void *)p;
-	if (!len) {
-		printf("%s: huh, no payload left\n", __func__);
-		nfs_free_packet(nfs_packet);
-		return NULL;
+	p = nfs_packet_read(nfs_packet, NFS3_COOKIEVERFSIZE);
+	if (!p) {
+		ret = -EINVAL;
+		goto err_free_packet;
 	}
 
-	buf = xzalloc(len);
-
-	memcpy(buf, p, len);
+	memcpy(dir->cookieverf, p, NFS3_COOKIEVERFSIZE);
 
-	nfs_free_packet(nfs_packet);
+	buf = nfs_packet_memdup_rest(nfs_packet, &len);
 
 	xdr_init(&dir->stream, buf, len);
 
+	ret = 0;
+
 	/* now xdr points to dirlist3 res.resok.reply */
+err_free_packet:
+	nfs_free_packet(nfs_packet);
 
-	return buf;
+	return ret ? NULL : buf;
 }
 
 /*
@@ -1000,19 +1121,37 @@ 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);
-	status = ntoh32(net_read_uint32(p++));
+	p = nfs_packet_read(nfs_packet, sizeof(uint32_t));
+	if (!p) {
+		ret = -EINVAL;
+		goto err_free_packet;
+	}
+
+	status = ntoh32(net_read_uint32(p));
 	if (status != NFS3_OK) {
 		pr_err("Read failed: %s\n", nfserrstr(status, &ret));
-		return ret;
+		goto err_free_packet;
 	}
 
-	p = nfs_read_post_op_attr(p, NULL);
+	ret = nfs_read_post_op_attr(nfs_packet, NULL);
+	if (ret) {
+		ret = -EINVAL;
+		goto err_free_packet;
+	}
+
+	p = nfs_packet_read(nfs_packet, sizeof(uint32_t));
+	if (!p) {
+		ret = -EINVAL;
+		goto err_free_packet;
+	}
 
 	rlen = ntoh32(net_read_uint32(p));
 
-	/* skip over count */
-	p += 1;
+	p = nfs_packet_read(nfs_packet, sizeof(uint32_t));
+	if (!p) {
+		ret = -EINVAL;
+		goto err_free_packet;
+	}
 
 	eof = ntoh32(net_read_uint32(p));
 
@@ -1020,18 +1159,23 @@ static int nfs_read_req(struct file_priv *priv, uint64_t offset,
 	 * skip over eof and count embedded in the representation of data
 	 * assuming it equals rlen above.
 	 */
-	p += 2;
+	nfs_packet_read(nfs_packet, sizeof(uint32_t));
 
 	if (readlen && !rlen && !eof) {
-		nfs_free_packet(nfs_packet);
-		return -EIO;
+		ret = -EIO;
+		goto err_free_packet;
 	}
 
+	p = nfs_packet_read(nfs_packet, rlen);
+
 	kfifo_put(priv->fifo, (char *)p, rlen);
 
+	ret = 0;
+
+err_free_packet:
 	nfs_free_packet(nfs_packet);
 
-	return 0;
+	return ret;
 }
 
 static void nfs_handler(void *ctx, char *p, unsigned len)
@@ -1043,6 +1187,7 @@ static void nfs_handler(void *ctx, char *p, unsigned len)
 	packet = xmalloc(sizeof(*packet) + len);
 	memcpy(packet->data, pkt, len);
 	packet->len = len;
+	packet->pos = 0;
 
 	list_add_tail(&packet->list, &npriv->packets);
 }
@@ -1066,6 +1211,7 @@ static int nfs_readlink_req(struct nfs_priv *npriv, struct nfs_fh *fh,
 	uint32_t data[1024];
 	uint32_t *p, status;
 	uint32_t len;
+	int ret;
 	struct packet *nfs_packet;
 
 	/*
@@ -1100,29 +1246,43 @@ 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);
-	status = ntoh32(net_read_uint32(p++));
+	p = nfs_packet_read(nfs_packet, sizeof(uint32_t));
+	if (!p) {
+		ret = -EINVAL;
+		goto err_free_packet;
+	}
+
+	status = ntoh32(net_read_uint32(p));
 	if (status != NFS3_OK) {
-		int ret;
 		pr_err("Readlink failed: %s\n", nfserrstr(status, &ret));
-		return ret;
+		goto err_free_packet;
 	}
 
-	p = nfs_read_post_op_attr(p, NULL);
+	nfs_read_post_op_attr(nfs_packet, NULL);
 
-	len = ntoh32(net_read_uint32(p)); /* new path length */
+	p = nfs_packet_read(nfs_packet, sizeof(uint32_t));
+	if (!p) {
+		ret = -EINVAL;
+		goto err_free_packet;
+	}
 
-	len = min_t(unsigned int, len,
-		    nfs_packet->len - sizeof(struct rpc_reply) - sizeof(uint32_t));
+	len = ntoh32(net_read_uint32(p)); /* new path length */
 
-	p++;
+	p = nfs_packet_read(nfs_packet, len);
+	if (!p) {
+		ret = -EINVAL;
+		goto err_free_packet;
+	}
 
 	*target = xzalloc(len + 1);
 	memcpy(*target, p, len);
 
+	ret = 0;
+
+err_free_packet:
 	nfs_free_packet(nfs_packet);
 
-	return 0;
+	return ret;
 }
 
 static const char *nfs_get_link(struct dentry *dentry, struct inode *inode)
-- 
2.47.3
^ permalink raw reply	[flat|nested] 5+ messages in thread
* [PATCH 3/4] fs: nfs: use dev_* for messages
  2025-11-04  8:08 [PATCH 0/4] fs: nfs: Fix buffer overflows Sascha Hauer
  2025-11-04  8:08 ` [PATCH 1/4] fs: nfs: drop PROG_NFS special casing Sascha Hauer
  2025-11-04  8:08 ` [PATCH 2/4] fs: nfs: do not read past packets Sascha Hauer
@ 2025-11-04  8:08 ` Sascha Hauer
  2025-11-04  8:08 ` [PATCH 4/4] fs: nfs: reduce unwanted message to debug level Sascha Hauer
  3 siblings, 0 replies; 5+ messages in thread
From: Sascha Hauer @ 2025-11-04  8:08 UTC (permalink / raw)
  To: BAREBOX
nfs uses a mixture of pr_*(), printf() and debug() functions for
messageds. Consistently use dev_*().
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 fs/nfs.c | 77 ++++++++++++++++++++++++++++++++++++++--------------------------
 1 file changed, 46 insertions(+), 31 deletions(-)
diff --git a/fs/nfs.c b/fs/nfs.c
index 163fd2182c0fc63d88725f0bba6ad5d1a71e2309..c9dc5d14ccec67e252750ce2bed076cf0b63cbd8 100644
--- a/fs/nfs.c
+++ b/fs/nfs.c
@@ -136,6 +136,7 @@ struct packet {
 };
 
 struct nfs_priv {
+	struct device *dev;
 	struct net_connection *con;
 	IPaddr_t server;
 	char *path;
@@ -366,8 +367,10 @@ static __be32 *xdr_inline_decode(struct xdr_stream *xdr, size_t nbytes)
 /*
  * name is expected to point to a buffer with a size of at least 256 bytes.
  */
-static int decode_filename(struct xdr_stream *xdr, char *name, u32 *length)
+static int decode_filename(struct nfs_priv *npriv, struct xdr_stream *xdr,
+			   char *name, u32 *length)
 {
+	struct device *dev = npriv->dev;
 	__be32 *p;
 	u32 count;
 
@@ -386,11 +389,11 @@ static int decode_filename(struct xdr_stream *xdr, char *name, u32 *length)
 	return 0;
 
 out_nametoolong:
-	pr_err("%s: returned a too long filename: %u\n", __func__, count);
+	dev_err(dev, "%s: returned a too long filename: %u\n", __func__, count);
 	return -ENAMETOOLONG;
 
 out_overflow:
-	pr_err("%s: premature end of packet\n", __func__);
+	dev_err(dev, "%s: premature end of packet\n", __func__);
 	return -EIO;
 }
 
@@ -502,6 +505,7 @@ static void nfs_free_packet(struct packet *packet)
 static struct packet *rpc_req(struct nfs_priv *npriv, int rpc_prog,
 			      int rpc_proc, uint32_t *data, int datalen)
 {
+	struct device *dev = npriv->dev;
 	struct rpc_call pkt;
 	unsigned short dport;
 	int ret;
@@ -517,7 +521,7 @@ static struct packet *rpc_req(struct nfs_priv *npriv, int rpc_prog,
 	pkt.prog = hton32(rpc_prog);
 	pkt.proc = hton32(rpc_proc);
 
-	debug("%s: prog: %d, proc: %d\n", __func__, rpc_prog, rpc_proc);
+	dev_dbg(dev, "%s: prog: %d, proc: %d\n", __func__, rpc_prog, rpc_proc);
 
 	if (rpc_prog == PROG_PORTMAP) {
 		dport = SUNRPC_PORT;
@@ -585,6 +589,7 @@ static struct packet *rpc_req(struct nfs_priv *npriv, int rpc_prog,
  */
 static int rpc_lookup_req(struct nfs_priv *npriv, uint32_t prog, uint32_t ver)
 {
+	struct device *dev = npriv->dev;
 	uint32_t data[16];
 	struct packet *nfs_packet;
 	uint32_t port;
@@ -613,7 +618,7 @@ static int rpc_lookup_req(struct nfs_priv *npriv, uint32_t prog, uint32_t ver)
 	nfs_free_packet(nfs_packet);
 
 	if (port == 0) {
-		pr_warn("No UDP port for RPC program %i! "
+		dev_warn(dev, "No UDP port for RPC program %i! "
 		        "Is your NFS server TCP only?\n", prog);
 		return -ENOENT;
 	}
@@ -686,8 +691,10 @@ static const struct {
 	{ 0x00800, S_ISUID },
 };
 
-static int nfs_fattr3_to_stat(uint32_t *p, struct inode *inode)
+static int nfs_fattr3_to_stat(struct nfs_priv *npriv, uint32_t *p,
+			      struct inode *inode)
 {
+	struct device *dev = npriv->dev;
 	uint32_t mode;
 	size_t i;
 
@@ -718,7 +725,7 @@ static int nfs_fattr3_to_stat(uint32_t *p, struct inode *inode)
 		inode->i_mode = S_IFIFO;
 		break;
 	default:
-		printf("%s: invalid mode %x\n",
+		dev_err(dev, "%s: invalid mode %x\n",
 				__func__, ntoh32(net_read_uint32(p + 0)));
 		return -EIO;
 	}
@@ -736,7 +743,8 @@ static int nfs_fattr3_to_stat(uint32_t *p, struct inode *inode)
 	return 0;
 }
 
-static int nfs_read_post_op_attr(struct packet *nfs_packet, struct inode *inode)
+static int nfs_read_post_op_attr(struct nfs_priv *npriv, struct packet *nfs_packet,
+				 struct inode *inode)
 {
 	void *p;
 
@@ -757,7 +765,7 @@ static int nfs_read_post_op_attr(struct packet *nfs_packet, struct inode *inode)
 		p = nfs_packet_read(nfs_packet, 21 * sizeof(uint32_t));
 		if (!p)
 			return -EINVAL;
-		nfs_fattr3_to_stat(p, inode);
+		nfs_fattr3_to_stat(npriv, p, inode);
 	}
 
 	return 0;
@@ -768,6 +776,7 @@ static int nfs_read_post_op_attr(struct packet *nfs_packet, struct inode *inode)
  */
 static int nfs_mount_req(struct nfs_priv *npriv)
 {
+	struct device *dev = npriv->dev;
 	uint32_t data[1024];
 	uint32_t *p, status;
 	int len;
@@ -777,7 +786,7 @@ static int nfs_mount_req(struct nfs_priv *npriv)
 
 	pathlen = strlen(npriv->path);
 
-	debug("%s: %s\n", __func__, npriv->path);
+	dev_dbg(dev, "%s: %s\n", __func__, npriv->path);
 
 	p = &(data[0]);
 	p = rpc_add_credentials(p);
@@ -807,7 +816,7 @@ static int nfs_mount_req(struct nfs_priv *npriv)
 
 	status = ntoh32(net_read_uint32(p));
 	if (status != NFS3_OK) {
-		pr_err("Mounting failed: %s\n", nfserrstr(status, &ret));
+		dev_err(dev, "Mounting failed: %s\n", nfserrstr(status, &ret));
 		goto err_free_packet;
 	}
 
@@ -819,7 +828,7 @@ static int nfs_mount_req(struct nfs_priv *npriv)
 
 	npriv->rootfh.size = ntoh32(net_read_uint32(p));
 	if (npriv->rootfh.size > NFS3_FHSIZE) {
-		printf("%s: file handle too big: %lu\n",
+		dev_err(dev, "%s: file handle too big: %lu\n",
 		       __func__, (unsigned long)npriv->rootfh.size);
 		ret = -EIO;
 		goto err_free_packet;
@@ -875,6 +884,7 @@ static void nfs_umount_req(struct nfs_priv *npriv)
 static int nfs_lookup_req(struct nfs_priv *npriv, struct nfs_fh *fh,
 			  const char *filename, struct inode *inode)
 {
+	struct device *dev = npriv->dev;
 	struct nfs_inode *ninode = nfsi(inode);
 	uint32_t data[1024];
 	uint32_t *p, status;
@@ -928,7 +938,7 @@ static int nfs_lookup_req(struct nfs_priv *npriv, struct nfs_fh *fh,
 
 	status = ntoh32(net_read_uint32(p));
 	if (status != NFS3_OK) {
-		pr_err("Lookup failed: %s\n", nfserrstr(status, &ret));
+		dev_err(dev, "Lookup failed: %s\n", nfserrstr(status, &ret));
 		goto err_free_packet;
 	}
 
@@ -940,7 +950,7 @@ static int nfs_lookup_req(struct nfs_priv *npriv, struct nfs_fh *fh,
 
 	ninode->fh.size = ntoh32(net_read_uint32(p));
 	if (ninode->fh.size > NFS3_FHSIZE) {
-		debug("%s: file handle too big: %u\n", __func__,
+		dev_dbg(dev, "%s: file handle too big: %u\n", __func__,
 		      ninode->fh.size);
 		ret = -EIO;
 		goto err_free_packet;
@@ -956,7 +966,7 @@ static int nfs_lookup_req(struct nfs_priv *npriv, struct nfs_fh *fh,
 
 	nfs_read_align(nfs_packet, 4);
 
-	nfs_read_post_op_attr(nfs_packet, inode);
+	nfs_read_post_op_attr(npriv, nfs_packet, inode);
 
 	ret = 0;
 
@@ -972,6 +982,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)
 {
+	struct device *dev = npriv->dev;
 	uint32_t data[1024];
 	uint32_t *p, status;
 	int len;
@@ -1040,12 +1051,12 @@ static void *nfs_readdirattr_req(struct nfs_priv *npriv, struct nfs_dir *dir)
 
 	status = ntoh32(net_read_uint32(p));
 	if (status != NFS3_OK) {
-		pr_err("Readdir failed: %s\n", nfserrstr(status, NULL));
+		dev_err(dev, "Readdir failed: %s\n", nfserrstr(status, NULL));
 		ret = -EIO;
 		goto err_free_packet;
 	}
 
-	ret = nfs_read_post_op_attr(nfs_packet, NULL);
+	ret = nfs_read_post_op_attr(npriv, nfs_packet, NULL);
 	if (ret)
 		goto err_free_packet;
 
@@ -1077,6 +1088,8 @@ static void *nfs_readdirattr_req(struct nfs_priv *npriv, struct nfs_dir *dir)
 static int nfs_read_req(struct file_priv *priv, uint64_t offset,
 		uint32_t readlen)
 {
+	struct nfs_priv *npriv = priv->npriv;
+	struct device *dev = npriv->dev;
 	uint32_t data[1024];
 	uint32_t *p, status;
 	int len, ret;
@@ -1129,11 +1142,11 @@ static int nfs_read_req(struct file_priv *priv, uint64_t offset,
 
 	status = ntoh32(net_read_uint32(p));
 	if (status != NFS3_OK) {
-		pr_err("Read failed: %s\n", nfserrstr(status, &ret));
+		dev_err(dev, "Read failed: %s\n", nfserrstr(status, &ret));
 		goto err_free_packet;
 	}
 
-	ret = nfs_read_post_op_attr(nfs_packet, NULL);
+	ret = nfs_read_post_op_attr(npriv, nfs_packet, NULL);
 	if (ret) {
 		ret = -EINVAL;
 		goto err_free_packet;
@@ -1208,6 +1221,7 @@ static void nfs_do_close(struct file_priv *priv)
 static int nfs_readlink_req(struct nfs_priv *npriv, struct nfs_fh *fh,
 			    char **target)
 {
+	struct device *dev = npriv->dev;
 	uint32_t data[1024];
 	uint32_t *p, status;
 	uint32_t len;
@@ -1254,11 +1268,11 @@ static int nfs_readlink_req(struct nfs_priv *npriv, struct nfs_fh *fh,
 
 	status = ntoh32(net_read_uint32(p));
 	if (status != NFS3_OK) {
-		pr_err("Readlink failed: %s\n", nfserrstr(status, &ret));
+		dev_err(dev, "Readlink failed: %s\n", nfserrstr(status, &ret));
 		goto err_free_packet;
 	}
 
-	nfs_read_post_op_attr(nfs_packet, NULL);
+	nfs_read_post_op_attr(npriv, nfs_packet, NULL);
 
 	p = nfs_packet_read(nfs_packet, sizeof(uint32_t));
 	if (!p) {
@@ -1408,7 +1422,7 @@ static int nfs_iterate(struct file *file, struct dir_context *ctx)
 			if (!p)
 				goto err_eop;
 
-			ret = decode_filename(xdr, name, &len);
+			ret = decode_filename(npriv, xdr, name, &len);
 			if (ret)
 				goto out;
 
@@ -1586,10 +1600,11 @@ static int nfs_probe(struct device *dev)
 	int ret;
 
 	dev->priv = npriv;
+	npriv->dev = dev;
 
 	INIT_LIST_HEAD(&npriv->packets);
 
-	debug("nfs: mount: %s\n", fsdev->backingstore);
+	dev_dbg(dev, "mount: %s\n", fsdev->backingstore);
 
 	path = strchr(tmp, ':');
 	if (!path) {
@@ -1603,11 +1618,11 @@ static int nfs_probe(struct device *dev)
 
 	ret = resolv(tmp, &npriv->server);
 	if (ret) {
-		printf("cannot resolve \"%s\": %pe\n", tmp, ERR_PTR(ret));
+		dev_err(dev, "cannot resolve \"%s\": %pe\n", tmp, ERR_PTR(ret));
 		goto err1;
 	}
 
-	debug("nfs: server: %s path: %s\n", tmp, npriv->path);
+	dev_dbg(dev, "server: %s path: %s\n", tmp, npriv->path);
 
 	npriv->con = net_udp_new(npriv->server, SUNRPC_PORT, nfs_handler, npriv);
 	if (IS_ERR(npriv->con)) {
@@ -1623,7 +1638,7 @@ static int nfs_probe(struct device *dev)
 		if (!npriv->mount_port) {
 			ret = rpc_lookup_req(npriv, PROG_MOUNT, 3);
 			if (ret < 0) {
-				printf("lookup mount port failed with %d\n", ret);
+				dev_err(dev, "lookup mount port failed with %d\n", ret);
 				goto err2;
 			}
 			npriv->mount_port = ret;
@@ -1635,7 +1650,7 @@ static int nfs_probe(struct device *dev)
 		if (!npriv->nfs_port) {
 			ret = rpc_lookup_req(npriv, PROG_NFS, 3);
 			if (ret < 0) {
-				printf("lookup nfs port failed with %d\n", ret);
+				dev_err(dev, "lookup nfs port failed with %d\n", ret);
 				goto err2;
 			}
 			npriv->nfs_port = ret;
@@ -1644,7 +1659,7 @@ static int nfs_probe(struct device *dev)
 		}
 	} else {
 		if (nfsport_default > U16_MAX) {
-			printf("invalid NFS port: %d\n", nfsport_default);
+			dev_err(dev, "invalid NFS port: %d\n", nfsport_default);
 			return -EINVAL;
 		}
 
@@ -1652,12 +1667,12 @@ static int nfs_probe(struct device *dev)
 		npriv->manual_nfs_port = npriv->manual_mount_port = 1;
 	}
 
-	debug("mount port: %hu\n", npriv->mount_port);
-	debug("nfs port: %d\n", npriv->nfs_port);
+	dev_dbg(dev, "mount port: %hu\n", npriv->mount_port);
+	dev_dbg(dev, "nfs port: %d\n", npriv->nfs_port);
 
 	ret = nfs_mount_req(npriv);
 	if (ret) {
-		printf("mounting failed with %d\n", ret);
+		dev_err(dev, "mounting failed with %d\n", ret);
 		goto err2;
 	}
 
-- 
2.47.3
^ permalink raw reply	[flat|nested] 5+ messages in thread
* [PATCH 4/4] fs: nfs: reduce unwanted message to debug level
  2025-11-04  8:08 [PATCH 0/4] fs: nfs: Fix buffer overflows Sascha Hauer
                   ` (2 preceding siblings ...)
  2025-11-04  8:08 ` [PATCH 3/4] fs: nfs: use dev_* for messages Sascha Hauer
@ 2025-11-04  8:08 ` Sascha Hauer
  3 siblings, 0 replies; 5+ messages in thread
From: Sascha Hauer @ 2025-11-04  8:08 UTC (permalink / raw)
  To: BAREBOX
Looking up a nonexistent file in NFS is a normal usecase and the caller
should handle it appropriately. Do not print a message from the NFS core
in this case.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 fs/nfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/nfs.c b/fs/nfs.c
index c9dc5d14ccec67e252750ce2bed076cf0b63cbd8..5c2476cd88634c37cf2cc9358c0482c03d180daa 100644
--- a/fs/nfs.c
+++ b/fs/nfs.c
@@ -938,7 +938,7 @@ static int nfs_lookup_req(struct nfs_priv *npriv, struct nfs_fh *fh,
 
 	status = ntoh32(net_read_uint32(p));
 	if (status != NFS3_OK) {
-		dev_err(dev, "Lookup failed: %s\n", nfserrstr(status, &ret));
+		dev_dbg(dev, "Lookup failed: %s\n", nfserrstr(status, &ret));
 		goto err_free_packet;
 	}
 
-- 
2.47.3
^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-11-04  8:08 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-11-04  8:08 [PATCH 0/4] fs: nfs: Fix buffer overflows Sascha Hauer
2025-11-04  8:08 ` [PATCH 1/4] fs: nfs: drop PROG_NFS special casing Sascha Hauer
2025-11-04  8:08 ` [PATCH 2/4] fs: nfs: do not read past packets Sascha Hauer
2025-11-04  8:08 ` [PATCH 3/4] fs: nfs: use dev_* for messages Sascha Hauer
2025-11-04  8:08 ` [PATCH 4/4] fs: nfs: reduce unwanted message to debug level Sascha Hauer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox