Nathan Lynch <[email protected]> writes: > Nathan Lynch <[email protected]> writes: >> Michael Ellerman <[email protected]> writes: >>> Nathan Lynch via B4 Relay <[email protected]> >>> 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/[email protected]/ >>> >>> 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
