From: Trond Myklebust <[email protected]>
Ensure the call to rpc_run_task() cannot fail by preallocating the
rpc_task.
Fixes: 910ad38697d9 ("NFS: Fix memory allocation in rpc_alloc_task()")
Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/unlink.c | 1 +
include/linux/nfs_xdr.h | 1 +
2 files changed, 2 insertions(+)
diff --git a/fs/nfs/unlink.c b/fs/nfs/unlink.c
index 5fa11e1aca4c..6f325e10056c 100644
--- a/fs/nfs/unlink.c
+++ b/fs/nfs/unlink.c
@@ -347,6 +347,7 @@ nfs_async_rename(struct inode *old_dir, struct inode *new_dir,
data = kzalloc(sizeof(*data), GFP_KERNEL);
if (data == NULL)
return ERR_PTR(-ENOMEM);
+ task_setup_data.task = &data->task;
task_setup_data.callback_data = data;
data->cred = get_current_cred();
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index 49ba486aea5f..2863e5a69c6a 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -1694,6 +1694,7 @@ struct nfs_unlinkdata {
struct nfs_renamedata {
struct nfs_renameargs args;
struct nfs_renameres res;
+ struct rpc_task task;
const struct cred *cred;
struct inode *old_dir;
struct dentry *old_dentry;
--
2.35.1
From: Trond Myklebust <[email protected]>
If the call to rpc_alloc_task() fails, then ensure that the calldata is
released, and that rpc_run_task() and rpc_run_bc_task() bail out early.
Reported-by: NeilBrown <[email protected]>
Fixes: 910ad38697d9 ("NFS: Fix memory allocation in rpc_alloc_task()")
Signed-off-by: Trond Myklebust <[email protected]>
---
net/sunrpc/clnt.c | 7 +++++++
net/sunrpc/sched.c | 5 +++++
2 files changed, 12 insertions(+)
diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index 6757b0fa5367..af0174d7ce5a 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -1127,6 +1127,8 @@ struct rpc_task *rpc_run_task(const struct rpc_task_setup *task_setup_data)
struct rpc_task *task;
task = rpc_new_task(task_setup_data);
+ if (IS_ERR(task))
+ return task;
if (!RPC_IS_ASYNC(task))
task->tk_flags |= RPC_TASK_CRED_NOREF;
@@ -1227,6 +1229,11 @@ struct rpc_task *rpc_run_bc_task(struct rpc_rqst *req)
* Create an rpc_task to send the data
*/
task = rpc_new_task(&task_setup_data);
+ if (IS_ERR(task)) {
+ xprt_free_bc_request(req);
+ return task;
+ }
+
xprt_init_bc_request(req, task);
task->tk_action = call_bc_encode;
diff --git a/net/sunrpc/sched.c b/net/sunrpc/sched.c
index b258b87a3ec2..7f70c1e608b7 100644
--- a/net/sunrpc/sched.c
+++ b/net/sunrpc/sched.c
@@ -1128,6 +1128,11 @@ struct rpc_task *rpc_new_task(const struct rpc_task_setup *setup_data)
if (task == NULL) {
task = rpc_alloc_task();
+ if (task == NULL) {
+ rpc_release_calldata(setup_data->callback_ops,
+ setup_data->callback_data);
+ return ERR_PTR(-ENOMEM);
+ }
flags = RPC_TASK_DYNAMIC;
}
--
2.35.1
From: Trond Myklebust <[email protected]>
The allocation is done with GFP_KERNEL, but it could still fail in a low
memory situation.
Fixes: 4a85a6a3320b ("SUNRPC: Handle TCP socket sends with kernel_sendpage() again")
Signed-off-by: Trond Myklebust <[email protected]>
---
net/sunrpc/svcsock.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
index 478f857cdaed..6ea3d87e1147 100644
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -1096,7 +1096,9 @@ static int svc_tcp_sendmsg(struct socket *sock, struct xdr_buf *xdr,
int ret;
*sentp = 0;
- xdr_alloc_bvec(xdr, GFP_KERNEL);
+ ret = xdr_alloc_bvec(xdr, GFP_KERNEL);
+ if (ret < 0)
+ return ret;
ret = kernel_sendmsg(sock, &msg, &rm, 1, rm.iov_len);
if (ret < 0)
--
2.35.1
On Thu, 2022-04-07 at 14:46 -0400, [email protected] wrote:
> From: Trond Myklebust <[email protected]>
>
> The allocation is done with GFP_KERNEL, but it could still fail in a
> low
> memory situation.
>
> Fixes: 4a85a6a3320b ("SUNRPC: Handle TCP socket sends with
> kernel_sendpage() again")
> Signed-off-by: Trond Myklebust <[email protected]>
> ---
> net/sunrpc/svcsock.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
> index 478f857cdaed..6ea3d87e1147 100644
> --- a/net/sunrpc/svcsock.c
> +++ b/net/sunrpc/svcsock.c
> @@ -1096,7 +1096,9 @@ static int svc_tcp_sendmsg(struct socket *sock,
> struct xdr_buf *xdr,
> int ret;
>
> *sentp = 0;
> - xdr_alloc_bvec(xdr, GFP_KERNEL);
> + ret = xdr_alloc_bvec(xdr, GFP_KERNEL);
> + if (ret < 0)
> + return ret;
>
> ret = kernel_sendmsg(sock, &msg, &rm, 1, rm.iov_len);
> if (ret < 0)
Chuck,
Do you mind if I send this and the 8/8 as part of the client pull
request? I saw this while I was digging through the code and separating
out the client and server uses of xdr_alloc_bvec().
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
[email protected]
> On Apr 7, 2022, at 3:23 PM, Trond Myklebust <[email protected]> wrote:
>
> On Thu, 2022-04-07 at 14:46 -0400, [email protected] wrote:
>> From: Trond Myklebust <[email protected]>
>>
>> The allocation is done with GFP_KERNEL, but it could still fail in a
>> low
>> memory situation.
>>
>> Fixes: 4a85a6a3320b ("SUNRPC: Handle TCP socket sends with
>> kernel_sendpage() again")
>> Signed-off-by: Trond Myklebust <[email protected]>
>> ---
>> net/sunrpc/svcsock.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
>> index 478f857cdaed..6ea3d87e1147 100644
>> --- a/net/sunrpc/svcsock.c
>> +++ b/net/sunrpc/svcsock.c
>> @@ -1096,7 +1096,9 @@ static int svc_tcp_sendmsg(struct socket *sock,
>> struct xdr_buf *xdr,
>> int ret;
>>
>> *sentp = 0;
>> - xdr_alloc_bvec(xdr, GFP_KERNEL);
>> + ret = xdr_alloc_bvec(xdr, GFP_KERNEL);
>> + if (ret < 0)
>> + return ret;
>>
>> ret = kernel_sendmsg(sock, &msg, &rm, 1, rm.iov_len);
>> if (ret < 0)
>
>
> Chuck,
>
> Do you mind if I send this and the 8/8 as part of the client pull
> request? I saw this while I was digging through the code and separating
> out the client and server uses of xdr_alloc_bvec().
I browsed through these a few minutes ago. I don't see any technical
issues. But as you're listed as a maintainer of the SUNRPC code, I
didn't think I needed to give explicit permission.
--
Chuck Lever