* [PATCH v3 2/3] fs/nfs: copy data from rpc replies to local storage
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 ` Uwe Kleine-König
2019-01-21 11:27 ` [PATCH v3 3/3] fs/nfs: stop using a global variable for nfs packet payload Uwe Kleine-König
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
2 siblings, 0 replies; 4+ messages in thread
From: Uwe Kleine-König @ 2019-01-21 11:27 UTC (permalink / raw)
To: barebox
The nfs code uses data provided to the packet handler after net_poll()
returned. But the life time of this data already ended when net_poll()
returns. Most of the time it is possible to get away here but on i.MX28
the data is overwritten since commit 82ec28929cc9 ("net: fec_imx: Do not
use DMA coherent memory for Rx buffers").
So the data from the packet is copied to a malloced buffer that needs
free()ing when the data is not used any more.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
fs/nfs.c | 31 ++++++++++++++++++++++++++-----
1 file changed, 26 insertions(+), 5 deletions(-)
diff --git a/fs/nfs.c b/fs/nfs.c
index 7695fd42ba13..fa0ab75b5609 100644
--- a/fs/nfs.c
+++ b/fs/nfs.c
@@ -440,7 +440,6 @@ again:
nfs_timer_start = get_time_ns();
nfs_state = STATE_START;
- nfs_packet = NULL;
while (nfs_state != STATE_DONE) {
if (ctrlc()) {
@@ -459,8 +458,10 @@ again:
ret = rpc_check_reply(nfs_packet, rpc_prog,
npriv->rpc_id, &nfserr);
if (!ret) {
- if (rpc_prog == PROG_NFS)
+ if (rpc_prog == PROG_NFS && nfserr) {
+ free(nfs_packet);
ret = nfserr;
+ }
break;
}
}
@@ -489,6 +490,9 @@ static int rpc_lookup_req(struct nfs_priv *npriv, uint32_t prog, uint32_t ver)
return ret;
port = ntoh32(net_read_uint32(nfs_packet + sizeof(struct rpc_reply)));
+
+ free(nfs_packet);
+
return port;
}
@@ -658,11 +662,14 @@ static int nfs_mount_req(struct nfs_priv *npriv)
if (npriv->rootfh.size > NFS3_FHSIZE) {
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);
+
return 0;
}
@@ -675,6 +682,7 @@ static void nfs_umount_req(struct nfs_priv *npriv)
uint32_t *p;
int len;
int pathlen;
+ int ret;
pathlen = strlen(npriv->path);
@@ -685,7 +693,9 @@ static void nfs_umount_req(struct nfs_priv *npriv)
len = p - &(data[0]);
- rpc_req(npriv, PROG_MOUNT, MOUNT_UMOUNT, data, len);
+ ret = rpc_req(npriv, PROG_MOUNT, MOUNT_UMOUNT, data, len);
+ if (!ret)
+ free(nfs_packet);
}
/*
@@ -753,6 +763,8 @@ static int nfs_lookup_req(struct nfs_priv *npriv, struct nfs_fh *fh,
nfs_read_post_op_attr(p, inode);
+ free(nfs_packet);
+
return 0;
}
@@ -831,6 +843,7 @@ static void *nfs_readdirattr_req(struct nfs_priv *npriv, struct nfs_dir *dir)
len = nfs_packet + nfs_len - (void *)p;
if (!len) {
printf("%s: huh, no payload left\n", __func__);
+ free(nfs_packet);
return NULL;
}
@@ -838,6 +851,8 @@ static void *nfs_readdirattr_req(struct nfs_priv *npriv, struct nfs_dir *dir)
memcpy(buf, p, len);
+ free(nfs_packet);
+
xdr_init(&dir->stream, buf, len);
/* now xdr points to dirlist3 res.resok.reply */
@@ -912,11 +927,15 @@ static int nfs_read_req(struct file_priv *priv, uint64_t offset,
*/
p += 2;
- if (readlen && !rlen && !eof)
+ if (readlen && !rlen && !eof) {
+ free(nfs_packet);
return -EIO;
+ }
kfifo_put(priv->fifo, (char *)p, rlen);
+ free(nfs_packet);
+
return 0;
}
@@ -925,7 +944,7 @@ static void nfs_handler(void *ctx, char *packet, unsigned len)
char *pkt = net_eth_to_udp_payload(packet);
nfs_state = STATE_DONE;
- nfs_packet = pkt;
+ nfs_packet = xmemdup(pkt, len);
nfs_len = len;
}
@@ -992,6 +1011,8 @@ static int nfs_readlink_req(struct nfs_priv *npriv, struct nfs_fh *fh,
*target = xzalloc(len + 1);
memcpy(*target, p, len);
+ free(nfs_packet);
+
return 0;
}
--
2.20.1
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v3 3/3] fs/nfs: stop using a global variable for nfs packet payload
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
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
2 siblings, 0 replies; 4+ messages in thread
From: Uwe Kleine-König @ 2019-01-21 11:27 UTC (permalink / raw)
To: barebox
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
^ permalink raw reply [flat|nested] 4+ messages in thread