From: Sascha Hauer <s.hauer@pengutronix.de>
To: BAREBOX <barebox@lists.infradead.org>
Subject: [PATCH 2/4] fs: nfs: do not read past packets
Date: Tue, 04 Nov 2025 09:08:06 +0100 [thread overview]
Message-ID: <20251104-nfs-v1-2-3e69e5c82a41@pengutronix.de> (raw)
In-Reply-To: <20251104-nfs-v1-0-3e69e5c82a41@pengutronix.de>
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
next prev parent reply other threads:[~2025-11-04 8:08 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20251104-nfs-v1-2-3e69e5c82a41@pengutronix.de \
--to=s.hauer@pengutronix.de \
--cc=barebox@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox