Critical Leonardo + Micro Bootkey conflicts

[NicoHood]

The way we write the Bootloader key in the ram is not reliable. You can destroy important data since you can revert the bootloader reset. With a 32u4 it might be very unlikely (because of that much ram) but still possible and then people are wondering about weird bugs. And with a 16u2 (500b ram) it is even more likely that this will happen if the reset is reverted.

Referring to this lines:
https://github.com/arduino/Arduino/blob/master/hardware/arduino/cores/arduino/CDC.cpp#L94-L108

So what we need to do is to ensure this address is really NOT USED.

3 Solutions i have in mind:
Reserve the Bootkey address so its not used in any case. See this for a possible solution:
http://electronics.stackexchange.com/questions/140516/where-to-put-bootloader-key-in-ram-what-address-is-the-last-ram-address

The other solution would be to force a reset in any case or add some other debounce routines.

The last might be the best but 'biggest' change. Also save the value to a fixed address, add this to the bootloader and Arduino core. Shrink the size to 8bit since its reserved anyways and move it to the beginning of the ram to not fragment data. I know you wont be happy about this since you need to update all Bootloaders and the USB Core.

I think the simplest option to fix this is the first. Someone should have thought about this first but well, it would be a mess to tell the customers now to update the bootloader and most just cant do it.
But what we should do for further boards is to use the first ram address 0x100 and a 8 bit value (option3). I will also use this for my 16u2/32u2 core that i will publish soon (also as pullup request maybe).

[NicoHood]

A possible solution can be found here (2nd option):
https://github.com/NicoHood/HID/blob/master/CDC.cpp#L135-L146

[NicoHood]

Also the Bootloader itself should be fixed. Because it starts the sketch when the USB clock is still on. A better way would be a complete watchdog reset with different fuses then (like HoodLoader2).

Is anyone willing to accept/improve that? I would work on this, but not if nobody cares and the work is for nothing.

[joshgoebel]

Is there some way for a sketch to override the 0x800 location in CDC.cpp (part of core). I can imagine various ways to fix this but all of them require changing that address in CDC to something else... is there any each way for a sketch (or library) to do this without having to create a whole custom Arduino core lib?

[joshgoebel]

It seems for a "default" Arduino program the top 2 bytes of the stack should always be usable anyways... __do_global_ctors makes a CALL to main, pushing the return address onto the stack. main will call setup then just go into a tight for (;;) loop calling loop() and will never return. So it seems that return address will technically not ever be needed. So since this is kind of a "rare" things anyways why not make the "magic" address the top of the stack vs the middle of active RAM?

It seems far less likely to corrupt user space by using the top 2 bytes of the stack vs 2 bytes in the middle of user RAM.

[NicoHood]

The implementation is here:
https://github.com/arduino/Arduino/blob/master/hardware/arduino/avr/cores/arduino/CDC.cpp#L97

That sounds like a good idea. The only problem we have is that we stick in an endless loop, waiting for the MCU to reset. They have added a revert of the reset because on Macs there were problems were the OS accidentally set and removed the dtr line as far as I understood. However I am not using this on my version anyways.

The bigger problem we have is that every Leonardo needs this update. And that there will be two version of the IDE core files. You could write those bytes to the stack top of course with function. But then you need access to the DTR states which is not possible with the current IDE core. In my HID core it would work with a Controle Line Event.

But if we not stick to compatibility, or at least think of a better solution for further boards, we could do something like that. It would be interesting how the bootloader should get those 2 bytes after the reset. Isnt the stack pointer resetted afterwards?

[joshgoebel]

But if we not stick to compatibility, or at least think of a better solution for further boards, we could do something like that. It would be interesting how the bootloader should get those 2 bytes after the reset. Isnt the stack pointer resetted afterwards?

The location would be fixed (depending on hardware ram size). The 2 bytes would always be the top 2 bytes of RAM (0xAFF on Leonardo). My point was that for all default Arduino compiles this should actually be "safe" since the main function is never returned from - making the 2 byte return address on the top of the stack actually memory that really never serves a purpose at all. IE, it's "free" - and doesn't require us to steal 2 bytes from anywhere else.

Good questions about backwards compatibility... some thorny issues there.

[NicoHood]

Thats great! Can you provide some code to see how it could work?
Ergo: cant we add a compiler flag to the main function that it never returns to save those 2 bytes on any other AVR?

[joshgoebel]

There isn't really any code to show. You'd just change the magic 0x800 address to 0xAFE, and you're done.

Ergo: cant we add a compiler flag to the main function that it never returns to save those 2 bytes on any other AVR?

It's only Arduino's main() that never returns. To the compiler main is still called and technically could return. There is code to handle exiting (technically just putting the processor into a loop). The compiler expects methods to return. If you really needed those 2 extra bytes of RAM you could of course provide your own main function and just move the stack pointer manually back to the top.

For most software there are easier ways to get 2 bytes back though.

[NicoHood]

is there a definition for 0xAFE aka the end of the stack? and why 0xAFE? I am not sure if this is correct.

[joshgoebel]

On Leonardo the top of ram (and beginning stack pointer) is 0xAFF, making 0xAFE the first address you could store a 2 byte sequence for the magic 0x7777 number currently used.

[NicoHood]

Were can I read up those values?
I also need this for 2kb ram, 1kb ram and 500b ram.

Edit: is RAMEND as definition related to this?