Skip to content

Commit 1ce8da3

Browse files
HadrienG2GabrielMajeri
HadrienG2
authored and
GabrielMajeri
committed
Graphics output protocol cleanup (#49)
* Hide reserved byte of BltPixel (else user can trivially change it) * Improve docs and memory safety * Reuse the casts we already have * Apply rustfmt
1 parent c3a2b2f commit 1ce8da3

File tree

1 file changed

+168
-63
lines changed

1 file changed

+168
-63
lines changed

‎src/proto/console/gop.rs

Lines changed: 168 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,8 @@ pub struct GraphicsOutput {
5454
}
5555

5656
impl GraphicsOutput {
57+
/// Returns information for an available graphics mode that the graphics
58+
/// device and the set of active video output devices supports.
5759
fn query_mode(&self, index: u32) -> Result<Mode> {
5860
let mut info_sz = 0;
5961
let mut info = ptr::null();
@@ -68,7 +70,7 @@ impl GraphicsOutput {
6870
})
6971
}
7072

71-
/// Returns information about available graphics modes and output devices.
73+
/// Returns information about all available graphics modes.
7274
pub fn modes<'a>(&'a self) -> impl Iterator<Item = Mode> + 'a {
7375
ModeIter {
7476
gop: self,
@@ -77,7 +79,8 @@ impl GraphicsOutput {
7779
}
7880
}
7981

80-
/// Sets the current graphics mode.
82+
/// Sets the video device into the specified mode, clearing visible portions
83+
/// of the output display to black.
8184
///
8285
/// This function **will** invalidate the current framebuffer and change the current mode.
8386
pub fn set_mode(&mut self, mode: &Mode) -> Result<()> {
@@ -94,61 +97,144 @@ impl GraphicsOutput {
9497
color,
9598
dest: (dest_x, dest_y),
9699
dims: (width, height),
97-
} => (self.blt)(
98-
self,
99-
&color as *const _ as usize,
100-
0,
101-
0,
102-
0,
103-
dest_x,
104-
dest_y,
105-
width,
106-
height,
107-
1,
108-
).into(),
100+
} => {
101+
self.check_framebuffer_region((dest_x, dest_y), (width, height));
102+
(self.blt)(
103+
self,
104+
&color as *const _ as usize,
105+
0,
106+
0,
107+
0,
108+
dest_x,
109+
dest_y,
110+
width,
111+
height,
112+
0,
113+
).into()
114+
}
109115
BltOp::VideoToBltBuffer {
110116
buffer,
111-
stride,
112117
src: (src_x, src_y),
113-
dest: (dest_x, dest_y),
118+
dest: dest_region,
114119
dims: (width, height),
115-
} => (self.blt)(
116-
self,
117-
buffer.as_mut_ptr() as usize,
118-
1,
119-
src_x,
120-
src_y,
121-
dest_x,
122-
dest_y,
123-
width,
124-
height,
125-
stride,
126-
).into(),
120+
} => {
121+
self.check_framebuffer_region((src_x, src_y), (width, height));
122+
self.check_blt_buffer_region(dest_region, (width, height), buffer.len());
123+
match dest_region {
124+
BltRegion::Full => (self.blt)(
125+
self,
126+
buffer.as_mut_ptr() as usize,
127+
1,
128+
src_x,
129+
src_y,
130+
0,
131+
0,
132+
width,
133+
height,
134+
0,
135+
).into(),
136+
BltRegion::SubRectangle {
137+
coords: (dest_x, dest_y),
138+
px_stride,
139+
} => (self.blt)(
140+
self,
141+
buffer.as_mut_ptr() as usize,
142+
1,
143+
src_x,
144+
src_y,
145+
dest_x,
146+
dest_y,
147+
width,
148+
height,
149+
px_stride * core::mem::size_of::<BltPixel>(),
150+
).into(),
151+
}
152+
}
127153
BltOp::BufferToVideo {
128154
buffer,
129-
stride,
130-
src: (src_x, src_y),
155+
src: src_region,
131156
dest: (dest_x, dest_y),
132157
dims: (width, height),
133-
} => (self.blt)(
134-
self,
135-
buffer.as_ptr() as usize,
136-
2,
137-
src_x,
138-
src_y,
139-
dest_x,
140-
dest_y,
141-
width,
142-
height,
143-
stride,
144-
).into(),
158+
} => {
159+
self.check_blt_buffer_region(src_region, (width, height), buffer.len());
160+
self.check_framebuffer_region((dest_x, dest_y), (width, height));
161+
match src_region {
162+
BltRegion::Full => (self.blt)(
163+
self,
164+
buffer.as_ptr() as usize,
165+
2,
166+
0,
167+
0,
168+
dest_x,
169+
dest_y,
170+
width,
171+
height,
172+
0,
173+
).into(),
174+
BltRegion::SubRectangle {
175+
coords: (src_x, src_y),
176+
px_stride,
177+
} => (self.blt)(
178+
self,
179+
buffer.as_ptr() as usize,
180+
2,
181+
src_x,
182+
src_y,
183+
dest_x,
184+
dest_y,
185+
width,
186+
height,
187+
px_stride * core::mem::size_of::<BltPixel>(),
188+
).into(),
189+
}
190+
}
145191
BltOp::VideoToVideo {
146192
src: (src_x, src_y),
147193
dest: (dest_x, dest_y),
148194
dims: (width, height),
149-
} => (self.blt)(
150-
self, 0usize, 3, src_x, src_y, dest_x, dest_y, width, height, 0,
151-
).into(),
195+
} => {
196+
self.check_framebuffer_region((src_x, src_y), (width, height));
197+
self.check_framebuffer_region((dest_x, dest_y), (width, height));
198+
(self.blt)(
199+
self, 0usize, 3, src_x, src_y, dest_x, dest_y, width, height, 0,
200+
).into()
201+
}
202+
}
203+
}
204+
205+
/// Memory-safety check for accessing a region of the framebuffer
206+
fn check_framebuffer_region(&self, coords: (usize, usize), dims: (usize, usize)) {
207+
let (width, height) = self.current_mode_info().resolution();
208+
assert!(
209+
coords.0.saturating_add(dims.0) <= width,
210+
"Horizontal framebuffer coordinate out of bounds"
211+
);
212+
assert!(
213+
coords.1.saturating_add(dims.1) <= height,
214+
"Vertical framebuffer coordinate out of bounds"
215+
);
216+
}
217+
218+
/// Memory-safety check for accessing a region of a user-provided buffer
219+
fn check_blt_buffer_region(&self, region: BltRegion, dims: (usize, usize), buf_length: usize) {
220+
match region {
221+
BltRegion::Full => assert!(
222+
dims.0.saturating_add(dims.1.saturating_mul(dims.0)) <= buf_length,
223+
"BltBuffer access out of bounds"
224+
),
225+
BltRegion::SubRectangle {
226+
coords: (x, y),
227+
px_stride,
228+
} => {
229+
assert!(
230+
x.saturating_add(dims.0) <= px_stride,
231+
"Horizontal BltBuffer coordinate out of bounds"
232+
);
233+
assert!(
234+
y.saturating_add(dims.1).saturating_mul(px_stride) <= buf_length,
235+
"Vertical BltBuffer coordinate out of bounds"
236+
);
237+
}
152238
}
153239
}
154240

@@ -165,6 +251,10 @@ impl GraphicsOutput {
165251
/// It is also the callers responsibilty to use volatile memory accesses,
166252
/// otherwise they could be optimized to nothing.
167253
pub unsafe fn frame_buffer(&mut self) -> &mut [u8] {
254+
assert!(
255+
self.mode.info.format != PixelFormat::BltOnly,
256+
"Cannot access the framebuffer in a Blt-only mode"
257+
);
168258
let data = self.mode.fb_address as *mut u8;
169259
let len = self.mode.fb_size;
170260

@@ -321,7 +411,7 @@ pub struct BltPixel {
321411
pub blue: u8,
322412
pub green: u8,
323413
pub red: u8,
324-
pub reserved: u8,
414+
_reserved: u8,
325415
}
326416

327417
impl BltPixel {
@@ -331,7 +421,7 @@ impl BltPixel {
331421
red,
332422
green,
333423
blue,
334-
reserved: 0,
424+
_reserved: 0,
335425
}
336426
}
337427
}
@@ -342,11 +432,30 @@ impl From<u32> for BltPixel {
342432
blue: (color & 0x00_00_FF) as u8,
343433
green: (color & 0x00_FF_00 >> 8) as u8,
344434
red: (color & 0xFF_00_00 >> 16) as u8,
345-
reserved: 0,
435+
_reserved: 0,
346436
}
347437
}
348438
}
349439

440+
/// Region of the BltBuffer which we are operating on
441+
///
442+
/// Some Blt operations can operate on either the full BltBuffer or a
443+
/// sub-rectangle of it, but require the stride to be known in the latter case.
444+
#[derive(Clone, Copy, Debug)]
445+
pub enum BltRegion {
446+
/// Operate on the full BltBuffer
447+
Full,
448+
449+
/// Operate on a sub-rectangle of the BltBuffer
450+
SubRectangle {
451+
/// Coordinate of the rectangle in the BltBuffer
452+
coords: (usize, usize),
453+
454+
/// Stride (length of each row of the BltBuffer) in **pixels**
455+
px_stride: usize,
456+
},
457+
}
458+
350459
/// Blit operation to perform.
351460
#[derive(Debug)]
352461
pub enum BltOp<'a> {
@@ -363,37 +472,33 @@ pub enum BltOp<'a> {
363472
VideoToBltBuffer {
364473
/// Buffer into which to copy data.
365474
buffer: &'a mut [BltPixel],
366-
/// The stride is the width / number of bytes in each row of the user-provided buffer.
367-
stride: usize,
368-
/// The coordinate of the source rectangle, in the frame buffer.
475+
/// Coordinates of the source rectangle, in the frame buffer.
369476
src: (usize, usize),
370-
/// The coordinate of the destination rectangle, in the user-provided buffer.
371-
dest: (usize, usize),
372-
/// The width / height of the rectangles.
477+
/// Location of the destination rectangle in the user-provided buffer
478+
dest: BltRegion,
479+
/// Width / height of the rectangles.
373480
dims: (usize, usize),
374481
},
375482
/// Write data from the buffer to the video rectangle.
376483
/// Delta must be the stride (count of bytes in a row) of the buffer.
377484
BufferToVideo {
378485
/// Buffer from which to copy data.
379486
buffer: &'a [BltPixel],
380-
/// The stride is the width / number of bytes in each row of the user-provided buffer.
381-
stride: usize,
382-
/// The coordinate of the source rectangle, in the user-provided buffer.
383-
src: (usize, usize),
384-
/// The coordinate of the destination rectangle, in the frame buffer.
487+
/// Location of the source rectangle in the user-provided buffer.
488+
src: BltRegion,
489+
/// Coordinates of the destination rectangle, in the frame buffer.
385490
dest: (usize, usize),
386-
/// The width / height of the rectangles.
491+
/// Width / height of the rectangles.
387492
dims: (usize, usize),
388493
},
389494
/// Copy from the source rectangle in video memory to
390495
/// the destination rectangle, also in video memory.
391496
VideoToVideo {
392-
/// The coordinate of the source rectangle, in the frame buffer.
497+
/// Coordinates of the source rectangle, in the frame buffer.
393498
src: (usize, usize),
394-
/// The coordinate of the destination rectangle, also in the frame buffer.
499+
/// Coordinates of the destination rectangle, also in the frame buffer.
395500
dest: (usize, usize),
396-
/// The width / height of the rectangles.
501+
/// Width / height of the rectangles.
397502
dims: (usize, usize),
398503
},
399504
}

0 commit comments

Comments
 (0)