Skip to content

Commit cd7796c

Browse files
committed
twim: disallow zero-length buffers.
Code for setting DMA buffers is deduplicated into fns set_tx_buffer, set_rx_buffer. Fixes #208
1 parent 4527dfa commit cd7796c

File tree

1 file changed

+70
-131
lines changed

1 file changed

+70
-131
lines changed

nrf-hal-common/src/twim.rs

Lines changed: 70 additions & 131 deletions
Original file line numberDiff line numberDiff line change
@@ -94,27 +94,17 @@ where
9494
Twim(twim)
9595
}
9696

97-
/// Write to an I2C slave.
98-
///
99-
/// The buffer must have a length of at most 255 bytes on the nRF52832
100-
/// and at most 65535 bytes on the nRF52840.
101-
pub fn write(&mut self, address: u8, buffer: &[u8]) -> Result<(), Error> {
97+
/// Set TX buffer, checking that it is in RAM and has suitable length.
98+
unsafe fn set_tx_buffer(&mut self, buffer: &[u8]) -> Result<(), Error> {
10299
slice_in_ram_or(buffer, Error::DMABufferNotInDataMemory)?;
103100

101+
if buffer.len() == 0 {
102+
return Err(Error::TxBufferZeroLength);
103+
}
104104
if buffer.len() > EASY_DMA_SIZE {
105105
return Err(Error::TxBufferTooLong);
106106
}
107107

108-
// Conservative compiler fence to prevent optimizations that do not
109-
// take in to account actions by DMA. The fence has been placed here,
110-
// before any DMA action has started.
111-
compiler_fence(SeqCst);
112-
113-
self.0
114-
.address
115-
.write(|w| unsafe { w.address().bits(address) });
116-
117-
// Set up the DMA write.
118108
self.0.txd.ptr.write(|w|
119109
// We're giving the register a pointer to the stack. Since we're
120110
// waiting for the I2C transaction to end before this stack pointer
@@ -132,6 +122,61 @@ where
132122
// values.
133123
unsafe { w.maxcnt().bits(buffer.len() as _) });
134124

125+
Ok(())
126+
}
127+
128+
/// Set RX buffer, checking that it has suitable length.
129+
unsafe fn set_rx_buffer(&mut self, buffer: &mut [u8]) -> Result<(), Error> {
130+
// NOTE: RAM slice check is not necessary, as a mutable
131+
// slice can only be built from data located in RAM.
132+
133+
if buffer.len() == 0 {
134+
return Err(Error::RxBufferZeroLength);
135+
}
136+
if buffer.len() > EASY_DMA_SIZE {
137+
return Err(Error::RxBufferTooLong);
138+
}
139+
140+
self.0.rxd.ptr.write(|w|
141+
// We're giving the register a pointer to the stack. Since we're
142+
// waiting for the I2C transaction to end before this stack pointer
143+
// becomes invalid, there's nothing wrong here.
144+
//
145+
// The PTR field is a full 32 bits wide and accepts the full range
146+
// of values.
147+
unsafe { w.ptr().bits(buffer.as_mut_ptr() as u32) });
148+
self.0.rxd.maxcnt.write(|w|
149+
// We're giving it the length of the buffer, so no danger of
150+
// accessing invalid memory. We have verified that the length of the
151+
// buffer fits in an `u8`, so the cast to the type of maxcnt
152+
// is also fine.
153+
//
154+
// Note that that nrf52840 maxcnt is a wider
155+
// type than a u8, so we use a `_` cast rather than a `u8` cast.
156+
// The MAXCNT field is thus at least 8 bits wide and accepts the
157+
// full range of values that fit in a `u8`.
158+
unsafe { w.maxcnt().bits(buffer.len() as _) });
159+
160+
Ok(())
161+
}
162+
163+
/// Write to an I2C slave.
164+
///
165+
/// The buffer must have a length of at most 255 bytes on the nRF52832
166+
/// and at most 65535 bytes on the nRF52840.
167+
pub fn write(&mut self, address: u8, buffer: &[u8]) -> Result<(), Error> {
168+
// Conservative compiler fence to prevent optimizations that do not
169+
// take in to account actions by DMA. The fence has been placed here,
170+
// before any DMA action has started.
171+
compiler_fence(SeqCst);
172+
173+
self.0
174+
.address
175+
.write(|w| unsafe { w.address().bits(address) });
176+
177+
// Set up the DMA write.
178+
unsafe { self.set_tx_buffer(buffer)? };
179+
135180
// Clear address NACK.
136181
self.0.errorsrc.write(|w| w.anack().bit(true));
137182

@@ -176,13 +221,6 @@ where
176221
/// The buffer must have a length of at most 255 bytes on the nRF52832
177222
/// and at most 65535 bytes on the nRF52840.
178223
pub fn read(&mut self, address: u8, buffer: &mut [u8]) -> Result<(), Error> {
179-
// NOTE: RAM slice check is not necessary, as a mutable slice can only be
180-
// built from data located in RAM.
181-
182-
if buffer.len() > EASY_DMA_SIZE {
183-
return Err(Error::RxBufferTooLong);
184-
}
185-
186224
// Conservative compiler fence to prevent optimizations that do not
187225
// take in to account actions by DMA. The fence has been placed here,
188226
// before any DMA action has started.
@@ -193,25 +231,7 @@ where
193231
.write(|w| unsafe { w.address().bits(address) });
194232

195233
// Set up the DMA read.
196-
self.0.rxd.ptr.write(|w|
197-
// We're giving the register a pointer to the stack. Since we're
198-
// waiting for the I2C transaction to end before this stack pointer
199-
// becomes invalid, there's nothing wrong here.
200-
//
201-
// The PTR field is a full 32 bits wide and accepts the full range
202-
// of values.
203-
unsafe { w.ptr().bits(buffer.as_mut_ptr() as u32) });
204-
self.0.rxd.maxcnt.write(|w|
205-
// We're giving it the length of the buffer, so no danger of
206-
// accessing invalid memory. We have verified that the length of the
207-
// buffer fits in an `u8`, so the cast to the type of maxcnt
208-
// is also fine.
209-
//
210-
// Note that that nrf52840 maxcnt is a wider
211-
// type than a u8, so we use a `_` cast rather than a `u8` cast.
212-
// The MAXCNT field is thus at least 8 bits wide and accepts the
213-
// full range of values that fit in a `u8`.
214-
unsafe { w.maxcnt().bits(buffer.len() as _) });
234+
unsafe { self.set_rx_buffer(buffer)? };
215235

216236
// Clear address NACK.
217237
self.0.errorsrc.write(|w| w.anack().bit(true));
@@ -263,18 +283,6 @@ where
263283
wr_buffer: &[u8],
264284
rd_buffer: &mut [u8],
265285
) -> Result<(), Error> {
266-
// NOTE: RAM slice check for `rd_buffer` is not necessary, as a mutable
267-
// slice can only be built from data located in RAM.
268-
slice_in_ram_or(wr_buffer, Error::DMABufferNotInDataMemory)?;
269-
270-
if wr_buffer.len() > EASY_DMA_SIZE {
271-
return Err(Error::TxBufferTooLong);
272-
}
273-
274-
if rd_buffer.len() > EASY_DMA_SIZE {
275-
return Err(Error::RxBufferTooLong);
276-
}
277-
278286
// Conservative compiler fence to prevent optimizations that do not
279287
// take in to account actions by DMA. The fence has been placed here,
280288
// before any DMA action has started.
@@ -284,44 +292,11 @@ where
284292
.address
285293
.write(|w| unsafe { w.address().bits(address) });
286294

287-
// Set up the DMA write.
288-
self.0.txd.ptr.write(|w|
289-
// We're giving the register a pointer to the stack. Since we're
290-
// waiting for the I2C transaction to end before this stack pointer
291-
// becomes invalid, there's nothing wrong here.
292-
//
293-
// The PTR field is a full 32 bits wide and accepts the full range
294-
// of values.
295-
unsafe { w.ptr().bits(wr_buffer.as_ptr() as u32) });
296-
self.0.txd.maxcnt.write(|w|
297-
// We're giving it the length of the buffer, so no danger of
298-
// accessing invalid memory. We have verified that the length of the
299-
// buffer fits in an `u8`, so the cast to `u8` is also fine.
300-
//
301-
// The MAXCNT field is 8 bits wide and accepts the full range of
302-
// values.
303-
unsafe { w.maxcnt().bits(wr_buffer.len() as _) });
304-
305-
// Set up the DMA read.
306-
self.0.rxd.ptr.write(|w|
307-
// We're giving the register a pointer to the stack. Since we're
308-
// waiting for the I2C transaction to end before this stack pointer
309-
// becomes invalid, there's nothing wrong here.
310-
//
311-
// The PTR field is a full 32 bits wide and accepts the full range
312-
// of values.
313-
unsafe { w.ptr().bits(rd_buffer.as_mut_ptr() as u32) });
314-
self.0.rxd.maxcnt.write(|w|
315-
// We're giving it the length of the buffer, so no danger of
316-
// accessing invalid memory. We have verified that the length of the
317-
// buffer fits in an `u8`, so the cast to the type of maxcnt
318-
// is also fine.
319-
//
320-
// Note that that nrf52840 maxcnt is a wider
321-
// type than a u8, so we use a `_` cast rather than a `u8` cast.
322-
// The MAXCNT field is thus at least 8 bits wide and accepts the
323-
// full range of values that fit in a `u8`.
324-
unsafe { w.maxcnt().bits(rd_buffer.len() as _) });
295+
// Set up DMA buffers.
296+
unsafe {
297+
self.set_tx_buffer(wr_buffer)?;
298+
self.set_rx_buffer(rd_buffer)?;
299+
}
325300

326301
// Clear address NACK.
327302
self.0.errorsrc.write(|w| w.anack().bit(true));
@@ -395,10 +370,6 @@ where
395370
tx_buffer: &[u8],
396371
rx_buffer: &mut [u8],
397372
) -> Result<(), Error> {
398-
if rx_buffer.len() > EASY_DMA_SIZE {
399-
return Err(Error::RxBufferTooLong);
400-
}
401-
402373
// Conservative compiler fence to prevent optimizations that do not
403374
// take in to account actions by DMA. The fence has been placed here,
404375
// before any DMA action has started.
@@ -409,25 +380,7 @@ where
409380
.write(|w| unsafe { w.address().bits(address) });
410381

411382
// Set up the DMA read.
412-
self.0.rxd.ptr.write(|w|
413-
// We're giving the register a pointer to the stack. Since we're
414-
// waiting for the I2C transaction to end before this stack pointer
415-
// becomes invalid, there's nothing wrong here.
416-
//
417-
// The PTR field is a full 32 bits wide and accepts the full range
418-
// of values.
419-
unsafe { w.ptr().bits(rx_buffer.as_mut_ptr() as u32) });
420-
self.0.rxd.maxcnt.write(|w|
421-
// We're giving it the length of the buffer, so no danger of
422-
// accessing invalid memory. We have verified that the length of the
423-
// buffer fits in an `u8`, so the cast to the type of maxcnt
424-
// is also fine.
425-
//
426-
// Note that that nrf52840 maxcnt is a wider
427-
// type than a u8, so we use a `_` cast rather than a `u8` cast.
428-
// The MAXCNT field is thus at least 8 bits wide and accepts the
429-
// full range of values that fit in a `u8`.
430-
unsafe { w.maxcnt().bits(rx_buffer.len() as _) });
383+
unsafe { self.set_rx_buffer(rx_buffer)? };
431384

432385
// Chunk write data.
433386
let wr_buffer = &mut [0; FORCE_COPY_BUFFER_SIZE][..];
@@ -436,23 +389,7 @@ where
436389
wr_buffer[..chunk.len()].copy_from_slice(chunk);
437390

438391
// Set up the DMA write.
439-
self.0.txd.ptr.write(|w|
440-
// We're giving the register a pointer to the stack. Since we're
441-
// waiting for the I2C transaction to end before this stack pointer
442-
// becomes invalid, there's nothing wrong here.
443-
//
444-
// The PTR field is a full 32 bits wide and accepts the full range
445-
// of values.
446-
unsafe { w.ptr().bits(wr_buffer.as_ptr() as u32) });
447-
448-
self.0.txd.maxcnt.write(|w|
449-
// We're giving it the length of the buffer, so no danger of
450-
// accessing invalid memory. We have verified that the length of the
451-
// buffer fits in an `u8`, so the cast to `u8` is also fine.
452-
//
453-
// The MAXCNT field is 8 bits wide and accepts the full range of
454-
// values.
455-
unsafe { w.maxcnt().bits(wr_buffer.len() as _) });
392+
unsafe { self.set_tx_buffer(wr_buffer)? };
456393

457394
// Start write operation.
458395
self.0.tasks_starttx.write(|w|
@@ -572,6 +509,8 @@ pub struct Pins {
572509
pub enum Error {
573510
TxBufferTooLong,
574511
RxBufferTooLong,
512+
TxBufferZeroLength,
513+
RxBufferZeroLength,
575514
Transmit,
576515
Receive,
577516
DMABufferNotInDataMemory,

0 commit comments

Comments
 (0)