Pages: [1]   Go Down
Author Topic: Bug in pinMode and digitalWrite - incorrect validation of pin parameter  (Read 560 times)
0 Members and 1 Guest are viewing this topic.
Global Moderator
Offline Offline
Brattain Member
*****
Karma: 452
Posts: 18694
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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:
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.
Logged

nr Bundaberg, Australia
Offline Offline
Tesla Member
***
Karma: 121
Posts: 8461
Scattered showers my arse -- Noah, 2348BC.
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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
« Last Edit: May 10, 2011, 09:15:15 am by Graynomad » Logged

Rob Gray aka the GRAYnomad www.robgray.com

nr Bundaberg, Australia
Offline Offline
Tesla Member
***
Karma: 121
Posts: 8461
Scattered showers my arse -- Noah, 2348BC.
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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

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

______
Rob
Logged

Rob Gray aka the GRAYnomad www.robgray.com

Global Moderator
Offline Offline
Brattain Member
*****
Karma: 452
Posts: 18694
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Well it should test for something like:

Code:
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:
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.
« Last Edit: May 10, 2011, 07:11:11 pm by Nick Gammon » Logged

nr Bundaberg, Australia
Offline Offline
Tesla Member
***
Karma: 121
Posts: 8461
Scattered showers my arse -- Noah, 2348BC.
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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
« Last Edit: May 10, 2011, 07:14:19 pm by Graynomad » Logged

Rob Gray aka the GRAYnomad www.robgray.com

Pages: [1]   Go Up
Jump to: