Suspicious kfree at the end of cm_write

Suspicious kfree at the end of cm_write

[Mark Langsdorf]

commit 03d1571d added an unconditional kfree() to the end of cm_write() 
in drivers/acpi/custom_method.c. I've been reviewing commits after the 
unm issue, and I think this code is wrong. If cm_write() is called with 
*ppos = 0 and count < table.length, the buf is kzalloc'd and immediately 
free'd. On subsequent calls to cm_write(), if cumulative count equals 
table.length, then the current contents of buf are passed to 
acpi_install_method. In the extremely unlikely case that buf has been 
reallocated and overwritten with a different but valid ACPI method, then 
some method other than the intended method could be installed. I'm not 
sure that this is a security issue but I don't think this code is correct.

It's a trivial patch to fix, but I'm not certain of my analysis. Could 
someone who is more familiar with cm_write review commit 03d1571d and 
tell me if I'm off base here?

--Mark Langsdorf

Re: Suspicious kfree at the end of cm_write

[Rafael J. Wysocki]

On Fri, Apr 23, 2021 at 4:38 PM Mark Langsdorf <mlangsdo@redhat.com> wrote:
>
> commit 03d1571d added an unconditional kfree() to the end of cm_write()
> in drivers/acpi/custom_method.c. I've been reviewing commits after the
> unm issue, and I think this code is wrong. If cm_write() is called with
> *ppos = 0 and count < table.length, the buf is kzalloc'd and immediately
> free'd. On subsequent calls to cm_write(), if cumulative count equals
> table.length, then the current contents of buf are passed to
> acpi_install_method. In the extremely unlikely case that buf has been
> reallocated and overwritten with a different but valid ACPI method, then
> some method other than the intended method could be installed. I'm not
> sure that this is a security issue but I don't think this code is correct.

The current code is not correct, so good catch!

If I'm not mistaken, it will lead to use-after-free on a subsequent
call to cm_write() it the current one does not write the entire
buffer.