Skip to content

Commit 939c004

Browse files
tannewttannewt
committed
Switch to a shared piece of code to compute start and length of a
buffer from start, end and length. The old code miscomputed length leading to writing and reading from memory past the end of the buffer. Consolidating the code should make it easier to get right everywhere.
1 parent 076ff82 commit 939c004

8 files changed

Lines changed: 111 additions & 72 deletions

File tree

‎atmel-samd/Makefile‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,7 @@ SRC_C = \
207207
lib/fatfs/ff.c \
208208
lib/fatfs/option/ccsbcs.c \
209209
lib/timeutils/timeutils.c \
210+
lib/utils/buffer_helper.c \
210211
lib/utils/context_manager_helpers.c \
211212
lib/utils/interrupt_char.c \
212213
lib/utils/pyexec.c \

‎esp8266/Makefile‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,7 @@ LIB_SRC_C = $(addprefix lib/,\
171171
mp-readline/readline.c \
172172
netutils/netutils.c \
173173
timeutils/timeutils.c \
174+
utils/buffer_helper.c \
174175
utils/context_manager_helpers.c \
175176
utils/pyexec.c \
176177
utils/pyhelp.c \

‎lib/utils/buffer_helper.c‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
/*
2+
* This file is part of the MicroPython project, http://micropython.org/
3+
*
4+
* The MIT License (MIT)
5+
*
6+
* Copyright (c) 2017 Scott Shawcroft for Adafruit Industries
7+
*
8+
* Permission is hereby granted, free of charge, to any person obtaining a copy
9+
* of this software and associated documentation files (the "Software"), to deal
10+
* in the Software without restriction, including without limitation the rights
11+
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
12+
* copies of the Software, and to permit persons to whom the Software is
13+
* furnished to do so, subject to the following conditions:
14+
*
15+
* The above copyright notice and this permission notice shall be included in
16+
* all copies or substantial portions of the Software.
17+
*
18+
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
19+
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
20+
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
21+
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
22+
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
23+
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
24+
* THE SOFTWARE.
25+
*/
26+
27+
#include "lib/utils/buffer_helper.h"
28+
29+
void normalize_buffer_bounds(int32_t* start, int32_t end, uint32_t* length) {
30+
if (end < 0) {
31+
end += *length;
32+
} else if (((uint32_t) end) > *length) {
33+
end = *length;
34+
}
35+
if (*start < 0) {
36+
*start += *length;
37+
}
38+
if (end < *start) {
39+
*length = 0;
40+
} else {
41+
*length = end - *start;
42+
}
43+
}

‎lib/utils/buffer_helper.h‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
/*
2+
* This file is part of the MicroPython project, http://micropython.org/
3+
*
4+
* The MIT License (MIT)
5+
*
6+
* Copyright (c) 2017 Scott Shawcroft for Adafruit Industries
7+
*
8+
* Permission is hereby granted, free of charge, to any person obtaining a copy
9+
* of this software and associated documentation files (the "Software"), to deal
10+
* in the Software without restriction, including without limitation the rights
11+
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
12+
* copies of the Software, and to permit persons to whom the Software is
13+
* furnished to do so, subject to the following conditions:
14+
*
15+
* The above copyright notice and this permission notice shall be included in
16+
* all copies or substantial portions of the Software.
17+
*
18+
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
19+
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
20+
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
21+
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
22+
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
23+
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
24+
* THE SOFTWARE.
25+
*/
26+
27+
#ifndef __MICROPY_INCLUDED_LIB_UTILS_BUFFER_HELPER_H__
28+
#define __MICROPY_INCLUDED_LIB_UTILS_BUFFER_HELPER_H__
29+
30+
#include <stdint.h>
31+
32+
void normalize_buffer_bounds(int32_t* start, int32_t end, uint32_t* length);
33+
34+
#endif // __MICROPY_INCLUDED_LIB_UTILS_BUFFER_HELPER_H__

‎shared-bindings/bitbangio/I2C.c‎

Lines changed: 10 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#include "shared-bindings/bitbangio/I2C.h"
3131
#include "shared-bindings/microcontroller/Pin.h"
3232

33+
#include "lib/utils/buffer_helper.h"
3334
#include "lib/utils/context_manager_helpers.h"
3435
#include "py/mperrno.h"
3536
#include "py/runtime.h"
@@ -172,21 +173,14 @@ STATIC mp_obj_t bitbangio_i2c_readfrom_into(size_t n_args, const mp_obj_t *pos_a
172173
check_lock(self);
173174
mp_buffer_info_t bufinfo;
174175
mp_get_buffer_raise(args[ARG_buffer].u_obj, &bufinfo, MP_BUFFER_WRITE);
175-
int32_t end = args[ARG_end].u_int;
176-
if (end < 0) {
177-
end += bufinfo.len;
178-
}
179-
uint32_t start = args[ARG_start].u_int;
180-
uint32_t len = end - start;
181-
if ((uint32_t) end < start) {
182-
len = 0;
183-
} else if (len > bufinfo.len) {
184-
len = bufinfo.len;
185-
}
176+
177+
int32_t start = args[ARG_start].u_int;
178+
uint32_t length = bufinfo.len;
179+
normalize_buffer_bounds(&start, args[ARG_end].u_int, &length);
186180
uint8_t status = shared_module_bitbangio_i2c_read(self,
187181
args[ARG_address].u_int,
188182
((uint8_t*)bufinfo.buf) + start,
189-
len);
183+
length);
190184
if (status != 0) {
191185
mp_raise_OSError(status);
192186
}
@@ -228,21 +222,13 @@ STATIC mp_obj_t bitbangio_i2c_writeto(size_t n_args, const mp_obj_t *pos_args, m
228222
mp_buffer_info_t bufinfo;
229223
mp_get_buffer_raise(args[ARG_buffer].u_obj, &bufinfo, MP_BUFFER_READ);
230224

231-
int32_t end = args[ARG_end].u_int;
232-
if (end < 0) {
233-
end += bufinfo.len;
234-
}
235-
uint32_t start = args[ARG_start].u_int;
236-
uint32_t len = end - start;
237-
if ((uint32_t) end < start) {
238-
len = 0;
239-
} else if (len > bufinfo.len) {
240-
len = bufinfo.len;
241-
}
225+
int32_t start = args[ARG_start].u_int;
226+
uint32_t length = bufinfo.len;
227+
normalize_buffer_bounds(&start, args[ARG_end].u_int, &length);
242228

243229
// do the transfer
244230
uint8_t status = shared_module_bitbangio_i2c_write(self, args[ARG_address].u_int,
245-
((uint8_t*) bufinfo.buf) + start, len, args[ARG_stop].u_bool);
231+
((uint8_t*) bufinfo.buf) + start, length, args[ARG_stop].u_bool);
246232
if (status != 0) {
247233
mp_raise_OSError(status);
248234
}

‎shared-bindings/bitbangio/SPI.c‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,7 @@ MP_DEFINE_CONST_FUN_OBJ_1(bitbangio_spi_unlock_obj, bitbangio_spi_obj_unlock);
185185
//|
186186
//| Write the data contained in ``buf``. Requires the SPI being locked.
187187
//|
188+
// TODO(tannewt): Add support for start and end kwargs.
188189
STATIC mp_obj_t bitbangio_spi_write(mp_obj_t self_in, mp_obj_t wr_buf) {
189190
mp_buffer_info_t src;
190191
mp_get_buffer_raise(wr_buf, &src, MP_BUFFER_READ);
@@ -203,6 +204,7 @@ MP_DEFINE_CONST_FUN_OBJ_2(bitbangio_spi_write_obj, bitbangio_spi_write);
203204
//|
204205
//| Read into the buffer specified by ``buf`` while writing zeroes. Requires the SPI being locked.
205206
//|
207+
// TODO(tannewt): Add support for start and end kwargs.
206208
STATIC mp_obj_t bitbangio_spi_readinto(size_t n_args, const mp_obj_t *args) {
207209
mp_buffer_info_t bufinfo;
208210
mp_get_buffer_raise(args[1], &bufinfo, MP_BUFFER_WRITE);

‎shared-bindings/busio/I2C.c‎

Lines changed: 11 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#include "shared-bindings/microcontroller/Pin.h"
3131
#include "shared-bindings/busio/I2C.h"
3232

33+
#include "lib/utils/buffer_helper.h"
3334
#include "lib/utils/context_manager_helpers.h"
3435
#include "py/runtime.h"
3536
//| .. currentmodule:: busio
@@ -187,18 +188,11 @@ STATIC mp_obj_t busio_i2c_readfrom_into(size_t n_args, const mp_obj_t *pos_args,
187188

188189
mp_buffer_info_t bufinfo;
189190
mp_get_buffer_raise(args[ARG_buffer].u_obj, &bufinfo, MP_BUFFER_WRITE);
190-
int32_t end = args[ARG_end].u_int;
191-
if (end < 0) {
192-
end += bufinfo.len;
193-
}
194-
uint32_t start = args[ARG_start].u_int;
195-
uint32_t len = end - start;
196-
if ((uint32_t) end < start) {
197-
len = 0;
198-
} else if (len > bufinfo.len) {
199-
len = bufinfo.len;
200-
}
201-
uint8_t status = common_hal_busio_i2c_read(self, args[ARG_address].u_int, ((uint8_t*)bufinfo.buf) + start, len);
191+
192+
int32_t start = args[ARG_start].u_int;
193+
uint32_t length = bufinfo.len;
194+
normalize_buffer_bounds(&start, args[ARG_end].u_int, &length);
195+
uint8_t status = common_hal_busio_i2c_read(self, args[ARG_address].u_int, ((uint8_t*)bufinfo.buf) + start, length);
202196
if (status != 0) {
203197
mp_raise_OSError(status);
204198
}
@@ -241,21 +235,14 @@ STATIC mp_obj_t busio_i2c_writeto(size_t n_args, const mp_obj_t *pos_args, mp_ma
241235
mp_buffer_info_t bufinfo;
242236
mp_get_buffer_raise(args[ARG_buffer].u_obj, &bufinfo, MP_BUFFER_READ);
243237

244-
int32_t end = args[ARG_end].u_int;
245-
if (end < 0) {
246-
end += bufinfo.len;
247-
}
248-
uint32_t start = args[ARG_start].u_int;
249-
uint32_t len = end - start;
250-
if ((uint32_t) end < start) {
251-
len = 0;
252-
} else if (len > bufinfo.len) {
253-
len = bufinfo.len;
254-
}
238+
239+
int32_t start = args[ARG_start].u_int;
240+
uint32_t length = bufinfo.len;
241+
normalize_buffer_bounds(&start, args[ARG_end].u_int, &length);
255242

256243
// do the transfer
257244
uint8_t status = common_hal_busio_i2c_write(self, args[ARG_address].u_int,
258-
((uint8_t*) bufinfo.buf) + start, len, args[ARG_stop].u_bool);
245+
((uint8_t*) bufinfo.buf) + start, length, args[ARG_stop].u_bool);
259246
if (status != 0) {
260247
mp_raise_OSError(status);
261248
}

‎shared-bindings/busio/SPI.c‎

Lines changed: 9 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
#include "shared-bindings/microcontroller/Pin.h"
3333
#include "shared-bindings/busio/SPI.h"
3434

35+
#include "lib/utils/buffer_helper.h"
3536
#include "lib/utils/context_manager_helpers.h"
3637
#include "py/mperrno.h"
3738
#include "py/nlr.h"
@@ -217,19 +218,11 @@ STATIC mp_obj_t busio_spi_write(size_t n_args, const mp_obj_t *pos_args, mp_map_
217218

218219
mp_buffer_info_t bufinfo;
219220
mp_get_buffer_raise(args[ARG_buffer].u_obj, &bufinfo, MP_BUFFER_READ);
220-
int32_t end = args[ARG_end].u_int;
221-
if (end < 0) {
222-
end += bufinfo.len;
223-
}
224-
uint32_t start = args[ARG_start].u_int;
225-
uint32_t len = end - start;
226-
if ((uint32_t) end < start) {
227-
len = 0;
228-
} else if (len > bufinfo.len) {
229-
len = bufinfo.len;
230-
}
221+
int32_t start = args[ARG_start].u_int;
222+
uint32_t length = bufinfo.len;
223+
normalize_buffer_bounds(&start, args[ARG_end].u_int, &length);
231224

232-
bool ok = common_hal_busio_spi_write(self, ((uint8_t*)bufinfo.buf) + start, len);
225+
bool ok = common_hal_busio_spi_write(self, ((uint8_t*)bufinfo.buf) + start, length);
233226
if (!ok) {
234227
mp_raise_OSError(MP_EIO);
235228
}
@@ -262,19 +255,11 @@ STATIC mp_obj_t busio_spi_readinto(size_t n_args, const mp_obj_t *pos_args, mp_m
262255

263256
mp_buffer_info_t bufinfo;
264257
mp_get_buffer_raise(args[ARG_buffer].u_obj, &bufinfo, MP_BUFFER_WRITE);
265-
int32_t end = args[ARG_end].u_int;
266-
if (end < 0) {
267-
end += bufinfo.len;
268-
}
269-
uint32_t start = args[ARG_start].u_int;
270-
uint32_t len = end - start;
271-
if ((uint32_t) end < start) {
272-
len = 0;
273-
} else if (len > bufinfo.len) {
274-
len = bufinfo.len;
275-
}
258+
int32_t start = args[ARG_start].u_int;
259+
uint32_t length = bufinfo.len;
260+
normalize_buffer_bounds(&start, args[ARG_end].u_int, &length);
276261

277-
bool ok = common_hal_busio_spi_read(self, ((uint8_t*)bufinfo.buf) + start, len, args[ARG_write_value].u_int);
262+
bool ok = common_hal_busio_spi_read(self, ((uint8_t*)bufinfo.buf) + start, length, args[ARG_write_value].u_int);
278263
if (!ok) {
279264
mp_raise_OSError(MP_EIO);
280265
}

0 commit comments

Comments
 (0)