| Message ID | 20210221035846.680145-1-ben.widawsky@intel.com |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] cxl/mem: Fix potential memory leak | expand |
On Sat, 20 Feb 2021 19:58:46 -0800 Ben Widawsky <ben.widawsky@intel.com> wrote: > When submitting a command for userspace, input and output payload bounce > buffers are allocated. For a given command, both input and output > buffers may exist and so when allocation of the input buffer fails, the > output buffer must be freed too. > > As far as I can tell, userspace can't easily exploit the leak to OOM a > machine unless the machine was already near OOM state. > > Fixes: 583fa5e71cae ("cxl/mem: Add basic IOCTL interface") > Reported-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > Signed-off-by: Ben Widawsky <ben.widawsky@intel.com> I'd probably have used a goto and label but I guess this works fine a well. FWIW on such a small patch. Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > --- > drivers/cxl/mem.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c > index df895bcca63a..244cb7d89678 100644 > --- a/drivers/cxl/mem.c > +++ b/drivers/cxl/mem.c > @@ -514,8 +514,10 @@ static int handle_mailbox_cmd_from_user(struct cxl_mem *cxlm, > if (cmd->info.size_in) { > mbox_cmd.payload_in = vmemdup_user(u64_to_user_ptr(in_payload), > cmd->info.size_in); > - if (IS_ERR(mbox_cmd.payload_in)) > + if (IS_ERR(mbox_cmd.payload_in)) { > + kvfree(mbox_cmd.payload_out); > return PTR_ERR(mbox_cmd.payload_in); > + } > } > > rc = cxl_mem_mbox_get(cxlm);
On Sat, Feb 20, 2021 at 07:58:46PM -0800, Ben Widawsky wrote: > When submitting a command for userspace, input and output payload bounce > buffers are allocated. For a given command, both input and output > buffers may exist and so when allocation of the input buffer fails, the > output buffer must be freed too. > > As far as I can tell, userspace can't easily exploit the leak to OOM a > machine unless the machine was already near OOM state. > > Fixes: 583fa5e71cae ("cxl/mem: Add basic IOCTL interface") > Reported-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> And lets add the other R-tag: Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Thank you for quick turn-around! > Signed-off-by: Ben Widawsky <ben.widawsky@intel.com> > --- > drivers/cxl/mem.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c > index df895bcca63a..244cb7d89678 100644 > --- a/drivers/cxl/mem.c > +++ b/drivers/cxl/mem.c > @@ -514,8 +514,10 @@ static int handle_mailbox_cmd_from_user(struct cxl_mem *cxlm, > if (cmd->info.size_in) { > mbox_cmd.payload_in = vmemdup_user(u64_to_user_ptr(in_payload), > cmd->info.size_in); > - if (IS_ERR(mbox_cmd.payload_in)) > + if (IS_ERR(mbox_cmd.payload_in)) { > + kvfree(mbox_cmd.payload_out); > return PTR_ERR(mbox_cmd.payload_in); > + } > } > > rc = cxl_mem_mbox_get(cxlm); > -- > 2.30.1 >
diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c index df895bcca63a..244cb7d89678 100644 --- a/drivers/cxl/mem.c +++ b/drivers/cxl/mem.c @@ -514,8 +514,10 @@ static int handle_mailbox_cmd_from_user(struct cxl_mem *cxlm, if (cmd->info.size_in) { mbox_cmd.payload_in = vmemdup_user(u64_to_user_ptr(in_payload), cmd->info.size_in); - if (IS_ERR(mbox_cmd.payload_in)) + if (IS_ERR(mbox_cmd.payload_in)) { + kvfree(mbox_cmd.payload_out); return PTR_ERR(mbox_cmd.payload_in); + } } rc = cxl_mem_mbox_get(cxlm);
When submitting a command for userspace, input and output payload bounce buffers are allocated. For a given command, both input and output buffers may exist and so when allocation of the input buffer fails, the output buffer must be freed too. As far as I can tell, userspace can't easily exploit the leak to OOM a machine unless the machine was already near OOM state. Fixes: 583fa5e71cae ("cxl/mem: Add basic IOCTL interface") Reported-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Signed-off-by: Ben Widawsky <ben.widawsky@intel.com> --- drivers/cxl/mem.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)