Go Down

Topic: Bug in pinMode and digitalWrite - incorrect validation of pin parameter (Read 749 times) previous topic - next topic

Nick Gammon

As discussed here:

http://arduino.cc/forum/index.php?topic=60252

There is a bug in the way that pinMode and digitalWrite work. They attempt to validate the supplied "pin" argument like this:

Code: [Select]
uint8_t port = digitalPinToPort(pin);
...
if (port == NOT_A_PIN) return;


However digitalPinToPort does not return NOT_A_PIN so this check does nothing.

The net result is that some random piece of memory will be written to in an attempt to set a pin mode, or write to a pin, if the pin is not in range.
Please post technical questions on the forum, not by personal message. Thanks!

More info:
http://www.gammon.com.au/electronics

Graynomad

What file are those functions (digitalPinToBitMask/digitalPinToPort) in Nick, I can't find the bloody things, and neither can grep.

EDIT: Never mind, it's a #define
Rob Gray aka the GRAYnomad www.robgray.com

Graynomad

Looks to me like the parameter is used to index directly into the digital_pin_to_port_PGM array with no checks.

Code: [Select]
extern const uint8_t PROGMEM digital_pin_to_port_PGM[];
....
#define digitalPinToPort(P) ( pgm_read_byte( digital_pin_to_port_PGM + (P) ) )


______
Rob
Rob Gray aka the GRAYnomad www.robgray.com

Nick Gammon

#3
May 10, 2011, 10:34 pm Last Edit: May 11, 2011, 02:11 am by Nick Gammon Reason: 1
Well it should test for something like:

Code: [Select]
if (pin >= sizeof digital_pin_to_port_PGM) return;


But it can't do that exactly because digital_pin_to_port_PGM is an extern array, and its size isn't know from the .h file. But something along those lines.

For your information, on my test run the corrupted addresses would be (with duplicates removed, and sorted):

Code: [Select]
0x0000, 0x0023, 0x0024, 0x0025, 0x0026, 0x0027, 0x0028, 0x0029, 0x002A,
0x00C6, 0x017A, 0x018B, 0x01AA, 0x01AB, 0x01AD, 0x0201, 0x0202, 0x023D,
0x027B, 0x0303, 0x031D, 0x0404, 0x0700, 0x0739, 0x0778, 0x07B1, 0x07D1,
0x1FFF, 0x2010, 0x24DD, 0x32AA, 0x3CC2, 0x59E8, 0x5DE6, 0x85E8, 0x85F9,
0x900F, 0x901F, 0x90EF, 0x912F, 0x9140, 0x91EF, 0x91FF, 0x921D, 0x92BF,
0x92EF, 0x92FF, 0x931F, 0x93DF, 0x93FF, 0x940E, 0x9731, 0xBE0F, 0xBE1F,
0xBFDE, 0xC004, 0xE011, 0xE040, 0xE06C, 0xE071, 0xE080, 0xE091, 0xE0B1,
0xE120, 0xE2AA, 0xEA8E, 0xF009, 0xF011, 0xF039


Some of them "look valid" (eg. 0x24) but are really a side-effect of attempting to modify (say) pin 45.
Please post technical questions on the forum, not by personal message. Thanks!

More info:
http://www.gammon.com.au/electronics

Graynomad

There should be an N_PINS or similar constant defined you would think.

There was a thread on AVR Freaks the other day about Arduino libraries and core code not being commercial strength and potentially flaky. This seems to support that point of view.
______
Rob
Rob Gray aka the GRAYnomad www.robgray.com

Go Up