[PATCH 2/4] fs: nfs: do not read past packets
Sascha Hauer
s.hauer at pengutronix.de
Tue Nov 4 00:08:06 PST 2025
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 at 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
More information about the barebox
mailing list