mail archive of the barebox mailing list
 help / color / mirror / Atom feed
From: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
To: barebox@lists.infradead.org
Subject: [PATCH v3 3/3] fs/nfs: stop using a global variable for nfs packet payload
Date: Mon, 21 Jan 2019 12:27:32 +0100	[thread overview]
Message-ID: <20190121112732.23772-3-u.kleine-koenig@pengutronix.de> (raw)
In-Reply-To: <20190121112732.23772-1-u.kleine-koenig@pengutronix.de>

The lifetime of the data provided by a global variable is hard to track.
So move the data pointer into the nfs_priv structure and let rpc_req
return the data to its callers which in turn are responsible to free it.

The callers are changed to use a local variable accordingly. This makes
it plausible that the previous commit catched all usages of the global
data.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 fs/nfs.c | 122 +++++++++++++++++++++++++++++--------------------------
 1 file changed, 64 insertions(+), 58 deletions(-)

diff --git a/fs/nfs.c b/fs/nfs.c
index fa0ab75b5609..07a82ec60277 100644
--- a/fs/nfs.c
+++ b/fs/nfs.c
@@ -102,9 +102,6 @@
 #define NFS3ERR_BADTYPE		10007
 #define NFS3ERR_JUKEBOX		10008
 
-static void *nfs_packet;
-static int nfs_len;
-
 struct rpc_call {
 	uint32_t id;
 	uint32_t type;
@@ -133,6 +130,11 @@ struct nfs_fh {
 	unsigned char data[NFS3_FHSIZE];
 };
 
+struct packet {
+	int len;
+	char data[];
+};
+
 struct nfs_priv {
 	struct net_connection *con;
 	IPaddr_t server;
@@ -143,6 +145,7 @@ struct nfs_priv {
 	unsigned manual_nfs_port:1;
 	uint32_t rpc_id;
 	struct nfs_fh rootfh;
+	struct packet *nfs_packet;
 };
 
 struct file_priv {
@@ -355,8 +358,8 @@ static uint32_t *rpc_add_credentials(uint32_t *p)
 	return p;
 }
 
-static int rpc_check_reply(unsigned char *pkt,
-		int rpc_prog, uint32_t rpc_id, int *nfserr)
+static int rpc_check_reply(struct packet *pkt, int rpc_prog,
+			   uint32_t rpc_id, int *nfserr)
 {
 	uint32_t *data;
 	struct rpc_reply rpc;
@@ -366,7 +369,7 @@ static int rpc_check_reply(unsigned char *pkt,
 	if (!pkt)
 		return -EAGAIN;
 
-	memcpy(&rpc, pkt, sizeof(rpc));
+	memcpy(&rpc, pkt->data, sizeof(rpc));
 
 	if (ntoh32(rpc.id) != rpc_id) {
 		if (rpc_id - ntoh32(rpc.id) == 1)
@@ -385,7 +388,7 @@ static int rpc_check_reply(unsigned char *pkt,
 	if (rpc_prog != PROG_NFS)
 		return 0;
 
-	data = (uint32_t *)(pkt + sizeof(struct rpc_reply));
+	data = (uint32_t *)(pkt->data + sizeof(struct rpc_reply));
 	*nfserr = ntoh32(net_read_uint32(data));
 	*nfserr = -*nfserr;
 
@@ -397,8 +400,8 @@ static int rpc_check_reply(unsigned char *pkt,
 /*
  * rpc_req - synchronous RPC request
  */
-static int rpc_req(struct nfs_priv *npriv, int rpc_prog, int rpc_proc,
-		uint32_t *data, int datalen)
+static struct packet *rpc_req(struct nfs_priv *npriv, int rpc_prog,
+			      int rpc_proc, uint32_t *data, int datalen)
 {
 	struct rpc_call pkt;
 	unsigned short dport;
@@ -442,31 +445,31 @@ again:
 	nfs_state = STATE_START;
 
 	while (nfs_state != STATE_DONE) {
-		if (ctrlc()) {
-			ret = -EINTR;
-			break;
-		}
+		if (ctrlc())
+			return ERR_PTR(-EINTR);
+
 		net_poll();
 
 		if (is_timeout(nfs_timer_start, NFS_TIMEOUT)) {
 			tries++;
 			if (tries == NFS_MAX_RESEND)
-				return -ETIMEDOUT;
+				return ERR_PTR(-ETIMEDOUT);
 			goto again;
 		}
 
-		ret = rpc_check_reply(nfs_packet, rpc_prog,
-				npriv->rpc_id, &nfserr);
+		ret = rpc_check_reply(npriv->nfs_packet, rpc_prog,
+				      npriv->rpc_id, &nfserr);
 		if (!ret) {
 			if (rpc_prog == PROG_NFS && nfserr) {
-				free(nfs_packet);
-				ret = nfserr;
+				free(npriv->nfs_packet);
+				return ERR_PTR(nfserr);
+			} else {
+				return npriv->nfs_packet;
 			}
-			break;
 		}
 	}
 
-	return ret;
+	return npriv->nfs_packet;
 }
 
 /*
@@ -475,7 +478,7 @@ again:
 static int rpc_lookup_req(struct nfs_priv *npriv, uint32_t prog, uint32_t ver)
 {
 	uint32_t data[16];
-	int ret;
+	struct packet *nfs_packet;
 	uint32_t port;
 
 	data[0] = 0; data[1] = 0;	/* auth credential */
@@ -485,11 +488,11 @@ static int rpc_lookup_req(struct nfs_priv *npriv, uint32_t prog, uint32_t ver)
 	data[6] = hton32(17);	/* IP_UDP */
 	data[7] = 0;
 
-	ret = rpc_req(npriv, PROG_PORTMAP, PORTMAP_GETPORT, data, 8);
-	if (ret)
-		return ret;
+	nfs_packet = rpc_req(npriv, PROG_PORTMAP, PORTMAP_GETPORT, data, 8);
+	if (IS_ERR(nfs_packet))
+		return PTR_ERR(nfs_packet);
 
-	port = ntoh32(net_read_uint32(nfs_packet + sizeof(struct rpc_reply)));
+	port = ntoh32(net_read_uint32(nfs_packet->data + sizeof(struct rpc_reply)));
 
 	free(nfs_packet);
 
@@ -634,7 +637,7 @@ static int nfs_mount_req(struct nfs_priv *npriv)
 	uint32_t *p;
 	int len;
 	int pathlen;
-	int ret;
+	struct packet *nfs_packet;
 
 	pathlen = strlen(npriv->path);
 
@@ -652,21 +655,20 @@ static int nfs_mount_req(struct nfs_priv *npriv)
 
 	len = p - &(data[0]);
 
-	ret = rpc_req(npriv, PROG_MOUNT, MOUNT_ADDENTRY, data, len);
-	if (ret)
-		return ret;
+	nfs_packet = rpc_req(npriv, PROG_MOUNT, MOUNT_ADDENTRY, data, len);
+	if (IS_ERR(nfs_packet))
+		return PTR_ERR(nfs_packet);
 
-	p = nfs_packet + sizeof(struct rpc_reply) + 4;
+	p = (void *)nfs_packet->data + sizeof(struct rpc_reply) + 4;
 
 	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);
+		printf("%s: file handle too big: %lu\n",
+		       __func__, (unsigned long)npriv->rootfh.size);
 		free(nfs_packet);
 		return -EIO;
 	}
 	memcpy(npriv->rootfh.data, p, npriv->rootfh.size);
-	p += DIV_ROUND_UP(npriv->rootfh.size, 4);
 
 	free(nfs_packet);
 
@@ -682,7 +684,7 @@ static void nfs_umount_req(struct nfs_priv *npriv)
 	uint32_t *p;
 	int len;
 	int pathlen;
-	int ret;
+	struct packet *nfs_packet;
 
 	pathlen = strlen(npriv->path);
 
@@ -693,8 +695,9 @@ static void nfs_umount_req(struct nfs_priv *npriv)
 
 	len = p - &(data[0]);
 
-	ret = rpc_req(npriv, PROG_MOUNT, MOUNT_UMOUNT, data, len);
-	if (!ret)
+	nfs_packet = rpc_req(npriv, PROG_MOUNT, MOUNT_UMOUNT, data, len);
+
+	if (!IS_ERR(nfs_packet))
 		free(nfs_packet);
 }
 
@@ -710,7 +713,7 @@ static int nfs_lookup_req(struct nfs_priv *npriv, struct nfs_fh *fh,
 	uint32_t data[1024];
 	uint32_t *p;
 	int len;
-	int ret;
+	struct packet *nfs_packet;
 
 	/*
 	 * struct LOOKUP3args {
@@ -746,11 +749,11 @@ static int nfs_lookup_req(struct nfs_priv *npriv, struct nfs_fh *fh,
 
 	len = p - &(data[0]);
 
-	ret = rpc_req(npriv, PROG_NFS, NFSPROC3_LOOKUP, data, len);
-	if (ret)
-		return ret;
+	nfs_packet = rpc_req(npriv, PROG_NFS, NFSPROC3_LOOKUP, data, len);
+	if (IS_ERR(nfs_packet))
+		return PTR_ERR(nfs_packet);
 
-	p = nfs_packet + sizeof(struct rpc_reply) + 4;
+	p = (void *)nfs_packet->data + sizeof(struct rpc_reply) + 4;
 
 	ninode->fh.size = ntoh32(net_read_uint32(p++));
 	if (ninode->fh.size > NFS3_FHSIZE) {
@@ -777,7 +780,7 @@ static void *nfs_readdirattr_req(struct nfs_priv *npriv, struct nfs_dir *dir)
 	uint32_t data[1024];
 	uint32_t *p;
 	int len;
-	int ret;
+	struct packet *nfs_packet;
 	void *buf;
 
 	/*
@@ -829,18 +832,18 @@ static void *nfs_readdirattr_req(struct nfs_priv *npriv, struct nfs_dir *dir)
 
 	p = nfs_add_uint32(p, 1024); /* count */
 
-	ret = rpc_req(npriv, PROG_NFS, NFSPROC3_READDIR, data, p - data);
-	if (ret)
+	nfs_packet = rpc_req(npriv, PROG_NFS, NFSPROC3_READDIR, data, p - data);
+	if (IS_ERR(nfs_packet))
 		return NULL;
 
-	p = nfs_packet + sizeof(struct rpc_reply) + 4;
+	p = (void *)nfs_packet->data + sizeof(struct rpc_reply) + 4;
 	p = nfs_read_post_op_attr(p, NULL);
 
 	/* update cookieverf */
 	memcpy(dir->cookieverf, p, NFS3_COOKIEVERFSIZE);
 	p += NFS3_COOKIEVERFSIZE / 4;
 
-	len = nfs_packet + nfs_len - (void *)p;
+	len = (void *)nfs_packet->data + nfs_packet->len - (void *)p;
 	if (!len) {
 		printf("%s: huh, no payload left\n", __func__);
 		free(nfs_packet);
@@ -869,7 +872,7 @@ static int nfs_read_req(struct file_priv *priv, uint64_t offset,
 	uint32_t data[1024];
 	uint32_t *p;
 	int len;
-	int ret;
+	struct packet *nfs_packet;
 	uint32_t rlen, eof;
 
 	/*
@@ -906,11 +909,11 @@ static int nfs_read_req(struct file_priv *priv, uint64_t offset,
 
 	len = p - &(data[0]);
 
-	ret = rpc_req(priv->npriv, PROG_NFS, NFSPROC3_READ, data, len);
-	if (ret)
-		return ret;
+	nfs_packet = rpc_req(priv->npriv, PROG_NFS, NFSPROC3_READ, data, len);
+	if (IS_ERR(nfs_packet))
+		return PTR_ERR(nfs_packet);
 
-	p = nfs_packet + sizeof(struct rpc_reply) + 4;
+	p = (void *)nfs_packet->data + sizeof(struct rpc_reply) + 4;
 
 	p = nfs_read_post_op_attr(p, NULL);
 
@@ -942,10 +945,13 @@ static int nfs_read_req(struct file_priv *priv, uint64_t offset,
 static void nfs_handler(void *ctx, char *packet, unsigned len)
 {
 	char *pkt = net_eth_to_udp_payload(packet);
+	struct nfs_priv *npriv = ctx;
 
 	nfs_state = STATE_DONE;
-	nfs_packet = xmemdup(pkt, len);
-	nfs_len = len;
+
+	npriv->nfs_packet = xmalloc(sizeof(*npriv->nfs_packet) + len);
+	memcpy(npriv->nfs_packet->data, pkt, len);
+	npriv->nfs_packet->len = len;
 }
 
 static int nfs_truncate(struct device_d *dev, FILE *f, ulong size)
@@ -967,7 +973,7 @@ static int nfs_readlink_req(struct nfs_priv *npriv, struct nfs_fh *fh,
 	uint32_t data[1024];
 	uint32_t *p;
 	uint32_t len;
-	int ret;
+	struct packet *nfs_packet;
 
 	/*
 	 * struct READLINK3args {
@@ -997,11 +1003,11 @@ static int nfs_readlink_req(struct nfs_priv *npriv, struct nfs_fh *fh,
 
 	len = p - &(data[0]);
 
-	ret = rpc_req(npriv, PROG_NFS, NFSPROC3_READLINK, data, len);
-	if (ret)
-		return ret;
+	nfs_packet = rpc_req(npriv, PROG_NFS, NFSPROC3_READLINK, data, len);
+	if (IS_ERR(nfs_packet))
+		return PTR_ERR(nfs_packet);
 
-	p = nfs_packet + sizeof(struct rpc_reply) + 4;
+	p = (void *)nfs_packet->data + sizeof(struct rpc_reply) + 4;
 
 	p = nfs_read_post_op_attr(p, NULL);
 
-- 
2.20.1


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

  parent reply	other threads:[~2019-01-21 11:27 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-21 11:27 [PATCH v3 1/3] fs/nfs: don't try to set nfs error code from mount rpc call Uwe Kleine-König
2019-01-21 11:27 ` [PATCH v3 2/3] fs/nfs: copy data from rpc replies to local storage Uwe Kleine-König
2019-01-21 11:27 ` Uwe Kleine-König [this message]
2019-01-22  7:25 ` [PATCH v3 1/3] fs/nfs: don't try to set nfs error code from mount rpc call 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=20190121112732.23772-3-u.kleine-koenig@pengutronix.de \
    --to=u.kleine-koenig@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