Re: [PATCH] powerpc/pseries: Enforce hcall result buffer validity and size

[Michael Ellerman <mpe@ellerman.id.au>]

Nathan Lynch <nathanl@linux.ibm.com> writes:
> Nathan Lynch <nathanl@linux.ibm.com> writes:
>> Michael Ellerman <mpe@ellerman.id.au> writes:
>>> Nathan Lynch via B4 Relay <devnull+nathanl.linux.ibm.com@kernel.org>
>>> writes:
>>>>
>>>> plpar_hcall(), plpar_hcall9(), and related functions expect callers to
>>>> provide valid result buffers of certain minimum size. Currently this
>>>> is communicated only through comments in the code and the compiler has
>>>> no idea.
>>>>
>>>> For example, if I write a bug like this:
>>>>
>>>>   long retbuf[PLPAR_HCALL_BUFSIZE]; // should be PLPAR_HCALL9_BUFSIZE
>>>>   plpar_hcall9(H_ALLOCATE_VAS_WINDOW, retbuf, ...);
>>>>
>>>> This compiles with no diagnostics emitted, but likely results in stack
>>>> corruption at runtime when plpar_hcall9() stores results past the end
>>>> of the array. (To be clear this is a contrived example and I have not
>>>> found a real instance yet.)
>>>
>>> We did have some real stack corruption bugs in the past.
>>>
>>> I referred to them in my previous (much uglier) attempt at a fix:
>>>
>>>   https://patchwork.ozlabs.org/project/linuxppc-dev/patch/1476780032-21643-2-git-send-email-mpe@ellerman.id.au/
>>>
>>> Annoyingly I didn't describe them in any detail, but at least one of them was:
>>>
>>>   24c65bc7037e ("hwrng: pseries - port to new read API and fix stack
>>>   corruption")
>>
>> Thanks for this background.
>>
>>
>>> Will this catch a case like that? Where the too-small buffer is not
>>> declared locally but rather comes into the function as a pointer?
>>
>> No, unfortunately. But here's a sketch that forces retbuf to be an
>> array [...]
>
> I've made some attempts to improve on this, but I think the original
> patch as written may be the best we can do without altering existing
> call sites or introducing new APIs and types.
...
>
> OK with taking the patch as-is?

Yeah. It's an improvement, even if it's not the full solution.

cheers