Sorry, you're of course right, guess I didn't get enough sleep last night… 9_9 Am 25.03.25 um 13:09 schrieb Michael Geddes:
That's to make sure a minimum width so that packages shorter than expected dont have issues with the functions accessing it beyond the buffer.
Oversize will just adapt anyway.
On Tue, 25 Mar 2025, 17:49 Michael Balzer via OvmsDev, <ovmsdev@lists.openvehicles.com> wrote:
I see you already checked the poll handlers don't write to the buffer, so that's a possible & better option.
But why do you limit the reserve() call? Clearly, the MAX_POLL_DATA_LEN isn't sufficient for the new poll, Wayne wants to add. Doing rxbuf.reserve(length + job.mlremain) should be perfectly OK, as it adapts to any reply size? After all, it's just a reservation, append will need to reallocate if it's too small. I think the module can get rid off MAX_POLL_DATA_LEN altogether when using the string memory directly.
Regards, Michael
Am 25.03.25 um 09:54 schrieb Michael Geddes via OvmsDev:
Why don't we just std:max() the reserve() on the string and cast c_str() to uint8_t * !? That way we know the array access in the code won't fumble into uncharted space and we get rid of the unchecked copy into the static memory space?
//.ichael
On Tue, 25 Mar 2025 at 16:38, Michael Balzer via OvmsDev <ovmsdev@lists.openvehicles.com> wrote:
Wayne,
uh, yes, that's a typical buffer overflow pattern there, unguarded copying of a string contents to a fixed size buffer:
> static uint8_t buf[MAX_POLL_DATA_LEN]; > memcpy(buf, rxbuf.c_str(), rxbuf.size());
Not sure why/if the handlers need an uint8_t array in the first place, but a quick first fix should be to adjust MAX_POLL_DATA_LEN:
> #define MAX_POLL_DATA_LEN 196
Add some spare room to the 329 bytes needed, just in case.
Regards, Michael
Am 25.03.25 um 09:31 schrieb Wayne Love: > Hi Micheal, > > Your comment... > >> Regarding Leaf CAN problems there ist a running investigation here: >> https://github.com/openvehicles/Open-Vehicle-Monitoring-System-3/issues/980 > > Is dead on the money. > > Polling group 61 returns an abnormally large response, 329 bytes. > This causes a buffer overrun in > OvmsVehicleNissanLeaf::IncomingPollReply with an unguarded memcpy that > causes the module to crash. Once the module crashes, I get the exact > symptoms in issue 980. > > Appreciate your help with this. > > Thanks > Wayne > > >
-- Michael Balzer * Am Rahmen 5 * D-58313 Herdecke <https://www.google.com/maps/search/Am+Rahmen+5+*+D-58313+Herdecke?entry=gmail&source=g> Fon 02330 9104094 * Handy 0176 20698926
_______________________________________________ OvmsDev mailing list OvmsDev@lists.openvehicles.com http://lists.openvehicles.com/mailman/listinfo/ovmsdev
_______________________________________________ OvmsDev mailing list OvmsDev@lists.openvehicles.com http://lists.openvehicles.com/mailman/listinfo/ovmsdev
-- Michael Balzer *Am Rahmen 5 * D-58313 Herdecke <https://www.google.com/maps/search/Am+Rahmen+5+*+D-58313+Herdecke?entry=gmail&source=g> Fon 02330 9104094 * Handy 0176 20698926
_______________________________________________ OvmsDev mailing list OvmsDev@lists.openvehicles.com http://lists.openvehicles.com/mailman/listinfo/ovmsdev
-- Michael Balzer * Am Rahmen 5 * D-58313 Herdecke Fon 02330 9104094 * Handy 0176 20698926