From: Anna Schumaker <[email protected]>
Instead of using a tiny, static scratch buffer, we should use a kmalloc()-ed
buffer that is allocated when checking for read plus usage. This lets us
use the buffer before decoding any part of the READ_PLUS operation
instead of setting it right before segment decoding, meaning it should
be a little more robust.
Signed-off-by: Anna Schumaker <[email protected]>
---
fs/nfs/nfs42xdr.c | 4 ++--
fs/nfs/nfs4proc.c | 17 ++++++++++++-----
include/linux/nfs_xdr.h | 1 +
3 files changed, 15 insertions(+), 7 deletions(-)
diff --git a/fs/nfs/nfs42xdr.c b/fs/nfs/nfs42xdr.c
index d80ee88ca996..a6df815a140c 100644
--- a/fs/nfs/nfs42xdr.c
+++ b/fs/nfs/nfs42xdr.c
@@ -1122,7 +1122,6 @@ static int decode_read_plus(struct xdr_stream *xdr, struct nfs_pgio_res *res)
uint32_t segments;
struct read_plus_segment *segs;
int status, i;
- char scratch_buf[16];
__be32 *p;
status = decode_op_hdr(xdr, OP_READ_PLUS);
@@ -1143,7 +1142,6 @@ static int decode_read_plus(struct xdr_stream *xdr, struct nfs_pgio_res *res)
if (!segs)
return -ENOMEM;
- xdr_set_scratch_buffer(xdr, &scratch_buf, sizeof(scratch_buf));
status = -EIO;
for (i = 0; i < segments; i++) {
status = decode_read_plus_segment(xdr, &segs[i]);
@@ -1348,6 +1346,8 @@ static int nfs4_xdr_dec_read_plus(struct rpc_rqst *rqstp,
struct compound_hdr hdr;
int status;
+ xdr_set_scratch_buffer(xdr, res->scratch, sizeof(res->scratch));
+
status = decode_compound_hdr(xdr, &hdr);
if (status)
goto out;
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 5607b1e2b821..18f25ff4bff7 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -5439,6 +5439,8 @@ static bool nfs4_read_plus_not_supported(struct rpc_task *task,
static int nfs4_read_done(struct rpc_task *task, struct nfs_pgio_header *hdr)
{
+ if (hdr->res.scratch)
+ kfree(hdr->res.scratch);
if (!nfs4_sequence_done(task, &hdr->res.seq_res))
return -EAGAIN;
if (nfs4_read_stateid_changed(task, &hdr->args))
@@ -5452,17 +5454,22 @@ static int nfs4_read_done(struct rpc_task *task, struct nfs_pgio_header *hdr)
}
#if defined CONFIG_NFS_V4_2 && defined CONFIG_NFS_V4_2_READ_PLUS
-static void nfs42_read_plus_support(struct nfs_pgio_header *hdr,
+static bool nfs42_read_plus_support(struct nfs_pgio_header *hdr,
struct rpc_message *msg)
{
/* Note: We don't use READ_PLUS with pNFS yet */
- if (nfs_server_capable(hdr->inode, NFS_CAP_READ_PLUS) && !hdr->ds_clp)
+ if (nfs_server_capable(hdr->inode, NFS_CAP_READ_PLUS) && !hdr->ds_clp) {
msg->rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_READ_PLUS];
+ hdr->res.scratch = kmalloc(32, GFP_KERNEL);
+ return hdr->res.scratch != NULL;
+ }
+ return false;
}
#else
-static void nfs42_read_plus_support(struct nfs_pgio_header *hdr,
+static bool nfs42_read_plus_support(struct nfs_pgio_header *hdr,
struct rpc_message *msg)
{
+ return false;
}
#endif /* CONFIG_NFS_V4_2 */
@@ -5472,8 +5479,8 @@ static void nfs4_proc_read_setup(struct nfs_pgio_header *hdr,
hdr->timestamp = jiffies;
if (!hdr->pgio_done_cb)
hdr->pgio_done_cb = nfs4_read_done_cb;
- msg->rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_READ];
- nfs42_read_plus_support(hdr, msg);
+ if (!nfs42_read_plus_support(hdr, msg))
+ msg->rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_READ];
nfs4_init_sequence(&hdr->args.seq_args, &hdr->res.seq_res, 0, 0);
}
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index e86cf6642d21..2fd973d188c4 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -670,6 +670,7 @@ struct nfs_pgio_res {
struct {
unsigned int replen; /* used by read */
int eof; /* used by read */
+ void * scratch; /* used by read */
};
struct {
struct nfs_writeverf * verf; /* used by write */
--
2.40.0
Sorry about the repost, I accidentally pointed a script to the wrong file!
Anna
On Fri, Apr 28, 2023 at 3:52 PM Anna Schumaker <[email protected]> wrote:
>
> From: Anna Schumaker <[email protected]>
>
> Instead of using a tiny, static scratch buffer, we should use a kmalloc()-ed
> buffer that is allocated when checking for read plus usage. This lets us
> use the buffer before decoding any part of the READ_PLUS operation
> instead of setting it right before segment decoding, meaning it should
> be a little more robust.
>
> Signed-off-by: Anna Schumaker <[email protected]>
> ---
> fs/nfs/nfs42xdr.c | 4 ++--
> fs/nfs/nfs4proc.c | 17 ++++++++++++-----
> include/linux/nfs_xdr.h | 1 +
> 3 files changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/fs/nfs/nfs42xdr.c b/fs/nfs/nfs42xdr.c
> index d80ee88ca996..a6df815a140c 100644
> --- a/fs/nfs/nfs42xdr.c
> +++ b/fs/nfs/nfs42xdr.c
> @@ -1122,7 +1122,6 @@ static int decode_read_plus(struct xdr_stream *xdr, struct nfs_pgio_res *res)
> uint32_t segments;
> struct read_plus_segment *segs;
> int status, i;
> - char scratch_buf[16];
> __be32 *p;
>
> status = decode_op_hdr(xdr, OP_READ_PLUS);
> @@ -1143,7 +1142,6 @@ static int decode_read_plus(struct xdr_stream *xdr, struct nfs_pgio_res *res)
> if (!segs)
> return -ENOMEM;
>
> - xdr_set_scratch_buffer(xdr, &scratch_buf, sizeof(scratch_buf));
> status = -EIO;
> for (i = 0; i < segments; i++) {
> status = decode_read_plus_segment(xdr, &segs[i]);
> @@ -1348,6 +1346,8 @@ static int nfs4_xdr_dec_read_plus(struct rpc_rqst *rqstp,
> struct compound_hdr hdr;
> int status;
>
> + xdr_set_scratch_buffer(xdr, res->scratch, sizeof(res->scratch));
> +
> status = decode_compound_hdr(xdr, &hdr);
> if (status)
> goto out;
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 5607b1e2b821..18f25ff4bff7 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -5439,6 +5439,8 @@ static bool nfs4_read_plus_not_supported(struct rpc_task *task,
>
> static int nfs4_read_done(struct rpc_task *task, struct nfs_pgio_header *hdr)
> {
> + if (hdr->res.scratch)
> + kfree(hdr->res.scratch);
> if (!nfs4_sequence_done(task, &hdr->res.seq_res))
> return -EAGAIN;
> if (nfs4_read_stateid_changed(task, &hdr->args))
> @@ -5452,17 +5454,22 @@ static int nfs4_read_done(struct rpc_task *task, struct nfs_pgio_header *hdr)
> }
>
> #if defined CONFIG_NFS_V4_2 && defined CONFIG_NFS_V4_2_READ_PLUS
> -static void nfs42_read_plus_support(struct nfs_pgio_header *hdr,
> +static bool nfs42_read_plus_support(struct nfs_pgio_header *hdr,
> struct rpc_message *msg)
> {
> /* Note: We don't use READ_PLUS with pNFS yet */
> - if (nfs_server_capable(hdr->inode, NFS_CAP_READ_PLUS) && !hdr->ds_clp)
> + if (nfs_server_capable(hdr->inode, NFS_CAP_READ_PLUS) && !hdr->ds_clp) {
> msg->rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_READ_PLUS];
> + hdr->res.scratch = kmalloc(32, GFP_KERNEL);
> + return hdr->res.scratch != NULL;
> + }
> + return false;
> }
> #else
> -static void nfs42_read_plus_support(struct nfs_pgio_header *hdr,
> +static bool nfs42_read_plus_support(struct nfs_pgio_header *hdr,
> struct rpc_message *msg)
> {
> + return false;
> }
> #endif /* CONFIG_NFS_V4_2 */
>
> @@ -5472,8 +5479,8 @@ static void nfs4_proc_read_setup(struct nfs_pgio_header *hdr,
> hdr->timestamp = jiffies;
> if (!hdr->pgio_done_cb)
> hdr->pgio_done_cb = nfs4_read_done_cb;
> - msg->rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_READ];
> - nfs42_read_plus_support(hdr, msg);
> + if (!nfs42_read_plus_support(hdr, msg))
> + msg->rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_READ];
> nfs4_init_sequence(&hdr->args.seq_args, &hdr->res.seq_res, 0, 0);
> }
>
> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
> index e86cf6642d21..2fd973d188c4 100644
> --- a/include/linux/nfs_xdr.h
> +++ b/include/linux/nfs_xdr.h
> @@ -670,6 +670,7 @@ struct nfs_pgio_res {
> struct {
> unsigned int replen; /* used by read */
> int eof; /* used by read */
> + void * scratch; /* used by read */
> };
> struct {
> struct nfs_writeverf * verf; /* used by write */
> --
> 2.40.0
>