Skip to content

Commit 1cf49df

Browse files
committed
Fix the soundness bug in the representation of extern types
Since the very first import dafaca9 ("Initial import of liblibc"), `libc` has used uninhabited enums to represent C's incomplete/opaque types. While this is, as far as I know, techincally okay when working behind raw pointers, it means that using reference types like `&FILE` can lead to easy UB. Resolve this by changing the representation to a `!Sync + !Send + !Unpin` ZST, as recommended by the nomicon [1]. The loss of auto traits technically makes this user-visible, but it is unlikely that anybody who is doing sound things was relying on these. I also used this as an opportunity to add a forward-compatibility note about intended use that should allow us to switch to real extern types once those are available. [1]: https://doc.rust-lang.org/nomicon/ffi.html#representing-opaque-structs
1 parent e3c4971 commit 1cf49df

24 files changed

Lines changed: 129 additions & 56 deletions

File tree

libc-test/build.rs

Lines changed: 64 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,9 @@ fn test_apple(target: &str) {
325325

326326
cfg.skip_struct(move |s| {
327327
match s.ident() {
328+
// Extern types
329+
"DIR" | "FILE" | "fpos_t" | "timezone" => true,
330+
328331
// FIXME(macos): The size is changed in recent macOSes.
329332
"malloc_zone_t" => true,
330333
// it is a moving target, changing through versions
@@ -572,6 +575,15 @@ fn test_openbsd(target: &str) {
572575
cfg.rename_struct_ty(|ty| ty.ends_with("_t").then_some(ty.to_string()));
573576
cfg.rename_union_ty(|ty| ty.ends_with("_t").then_some(ty.to_string()));
574577

578+
cfg.skip_struct(move |struct_| {
579+
match struct_.ident() {
580+
// Extern types
581+
"DIR" | "FILE" | "fpos_t" => true,
582+
583+
_ => false,
584+
}
585+
});
586+
575587
cfg.skip_struct_field(move |struct_, field| {
576588
match (struct_.ident(), field.ident()) {
577589
// conflicting with `p_type` macro from <resolve.h>.
@@ -693,6 +705,15 @@ fn test_cygwin(target: &str) {
693705
_ => false,
694706
});
695707

708+
cfg.skip_struct(move |struct_| {
709+
match struct_.ident() {
710+
// Extern types
711+
"DIR" | "FILE" | "fpos_t" => true,
712+
713+
_ => false,
714+
}
715+
});
716+
696717
cfg.rename_struct_field(move |struct_, field| {
697718
match field.ident() {
698719
// Our stat *_nsec fields normally don't actually exist but are part
@@ -828,6 +849,8 @@ fn test_windows(target: &str) {
828849
match struct_.ident() {
829850
// FIXME(windows): The size and alignment of this struct are incorrect
830851
"timespec" if gnu && i686 => true,
852+
// Extern types
853+
"FILE" | "fpos_t" | "timezone" => true,
831854
_ => false,
832855
}
833856
});
@@ -1089,13 +1112,16 @@ fn test_solarish(target: &str) {
10891112
false
10901113
});
10911114
cfg.skip_struct(move |struct_| {
1092-
// the union handling is a mess
1093-
if struct_.ident().contains("door_desc_t_") {
1094-
return true;
1095-
}
10961115
match struct_.ident() {
1116+
// the union handling is a mess
1117+
x if x.contains("door_desc_t_") => true,
1118+
10971119
// a bunch of solaris-only fields
10981120
"utmpx" if is_illumos => true,
1121+
1122+
// Extern types
1123+
"DIR" | "FILE" | "fpos_t" | "timezone" | "ucred_t" => true,
1124+
10991125
_ => false,
11001126
}
11011127
});
@@ -1362,6 +1388,8 @@ fn test_netbsd(target: &str) {
13621388
"ptrace_lwpinfo" => true,
13631389
// ABI change in NetBSD10, with symbol versioning.
13641390
"statvfs" if !netbsd9 => true,
1391+
// Extern types
1392+
"DIR" | "FILE" | "fpos_t" | "timezone" | "_cpuset" | "sem" => true,
13651393
_ => false,
13661394
}
13671395
});
@@ -1647,6 +1675,9 @@ fn test_dragonflybsd(target: &str) {
16471675
// structs.
16481676
"termios2" => true,
16491677

1678+
// Extern types
1679+
"DIR" | "FILE" | "fpos_t" => true,
1680+
16501681
_ => false,
16511682
}
16521683
});
@@ -1793,6 +1824,14 @@ fn test_wasi(target: &str) {
17931824
_ => false,
17941825
});
17951826

1827+
cfg.skip_struct(move |struct_| {
1828+
match struct_.ident() {
1829+
// Extern types
1830+
"DIR" | "FILE" | "__locale_struct" => true,
1831+
_ => false,
1832+
}
1833+
});
1834+
17961835
cfg.skip_fn(|f| match f.ident() {
17971836
// This function doesn't actually exist in libc's header files
17981837
"__errno_location" => true,
@@ -2041,6 +2080,9 @@ fn test_android(target: &str) {
20412080
// FIXME(android): The field has been changed:
20422081
"sockaddr_vm" => true,
20432082

2083+
// Extern types
2084+
"DIR" | "FILE" | "fpos_t" | "timezone" => true,
2085+
20442086
_ => false,
20452087
}
20462088
});
@@ -2846,6 +2888,9 @@ fn test_freebsd(target: &str) {
28462888
// Those are introduced in FreeBSD 15.
28472889
"xktls_session_onedir" | "xktls_session" if Some(15) > freebsd_ver => true,
28482890

2891+
// Extern types
2892+
"DIR" | "FILE" | "fpos_t" | "timezone" => true,
2893+
28492894
_ => false,
28502895
}
28512896
});
@@ -3144,6 +3189,9 @@ fn test_emscripten(target: &str) {
31443189
"pthread_condattr_t" => true,
31453190
"pthread_mutexattr_t" => true,
31463191

3192+
// Extern types
3193+
"DIR" | "FILE" | "fpos_t" | "fpos64_t" | "timezone" => true,
3194+
31473195
// No epoll support
31483196
// https://github.com/emscripten-core/emscripten/issues/5033
31493197
ty if ty.starts_with("epoll") => true,
@@ -3424,6 +3472,9 @@ fn test_neutrino(target: &str) {
34243472
// union
34253473
"_channel_connect_attr" => true,
34263474

3475+
// Extern types
3476+
"DIR" | "FILE" | "fpos_t" => true,
3477+
34273478
_ => false,
34283479
}
34293480
});
@@ -4221,9 +4272,10 @@ fn test_linux(target: &str) {
42214272
// On 64 bits the size did not change, skip only for 32 bits.
42224273
"ptrace_syscall_info" if pointer_width == 32 => true,
42234274

4275+
// not in sys/fanotify.h in uclibc
4276+
"fanotify_event_info_header" | "fanotify_event_info_fid" if uclibc => true,
4277+
42244278
"canxl_frame"
4225-
| "fanotify_event_info_header" // not in sys/fanotify.h in uclibc
4226-
| "fanotify_event_info_fid" // not in sys/fanotify.h in uclibc
42274279
| "tls12_crypto_info_sm4_gcm"
42284280
| "tls12_crypto_info_sm4_ccm"
42294281
| "tls12_crypto_info_aria_gcm_128"
@@ -4233,6 +4285,9 @@ fn test_linux(target: &str) {
42334285
true
42344286
}
42354287

4288+
// Extern types
4289+
"DIR" | "FILE" | "fpos_t" | "timezone" => true,
4290+
42364291
_ => false,
42374292
}
42384293
});
@@ -5734,6 +5789,9 @@ fn test_aix(target: &str) {
57345789
// header.
57355790
"fileops_t" | "file" => true,
57365791

5792+
// Extern types
5793+
"DIR" | "FILE" | "fpos_t" => true,
5794+
57375795
_ => false,
57385796
}
57395797
});

src/fuchsia/mod.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -79,9 +79,9 @@ pub type fsfilcnt_t = c_ulonglong;
7979
pub type rlim_t = c_ulonglong;
8080

8181
extern_ty! {
82-
pub enum timezone {}
83-
pub enum DIR {}
84-
pub enum fpos64_t {} // FIXME(fuchsia): fill this out with a struct
82+
pub type timezone;
83+
pub type DIR;
84+
pub type fpos64_t; // FIXME(fuchsia): fill this out with a struct
8585
}
8686

8787
// PUB_STRUCT
@@ -3188,8 +3188,8 @@ fn __MHDR_END(mhdr: *const msghdr) -> *mut c_uchar {
31883188
extern "C" {}
31893189

31903190
extern_ty! {
3191-
pub enum FILE {}
3192-
pub enum fpos_t {} // FIXME(fuchsia): fill this out with a struct
3191+
pub type FILE;
3192+
pub type fpos_t; // FIXME(fuchsia): fill this out with a struct
31933193
}
31943194

31953195
extern "C" {

src/macros.rs

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -246,17 +246,36 @@ macro_rules! s_no_extra_traits {
246246
macro_rules! extern_ty {
247247
($(
248248
$(#[$attr:meta])*
249-
pub enum $i:ident {}
249+
pub type $i:ident;
250250
)*) => ($(
251251
$(#[$attr])*
252-
// FIXME(1.0): the type is uninhabited so these traits are unreachable and could be
253-
// removed.
252+
/// This is an extern type ("opaque" or "incomplete" type in C).
253+
///
254+
/// <div class="warning">
255+
/// This type's current representation allows inspecting some properties, such as via
256+
/// <code>size_of</code>, and it is technically possible to construct the type within
257+
/// <code>MaybeUninit</code>, However, this <strong>MUST NOT</strong> be relied upon
258+
/// because a future version of <code>libc</code> may switch to a proper
259+
/// <a href="https://rust-lang.github.io/rfcs/1861-extern-types.html">extern type</a>
260+
/// representation when available.
261+
/// </div>
262+
// ^ unfortunately warning blocks currently don't render markdown so we need to
263+
// use raw HTML.
264+
//
265+
// Representation based on the Nomicon:
266+
// <https://doc.rust-lang.org/nomicon/ffi.html#representing-opaque-structs>.
267+
//
268+
// FIXME(1.0): These traits are unreachable and should be removed.
254269
#[::core::prelude::v1::derive(
255270
::core::clone::Clone,
256271
::core::marker::Copy,
257272
::core::fmt::Debug,
258273
)]
259-
pub enum $i { }
274+
#[repr(C)]
275+
pub struct $i {
276+
_data: (),
277+
_marker: ::core::marker::PhantomData<(*mut u8, ::core::marker::PhantomPinned)>,
278+
}
260279
)*);
261280
}
262281

src/new/qurt/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ pub type in_port_t = c_ushort;
6363

6464
// Standard C library types
6565
extern_ty! {
66-
pub enum FILE {}
66+
pub type FILE;
6767
}
6868
pub type fpos_t = c_long;
6969
pub type clock_t = c_long;

src/solid/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -397,8 +397,8 @@ pub const SIGUSR2: c_int = 31;
397397
pub const SIGPWR: c_int = 32;
398398

399399
extern_ty! {
400-
pub enum FILE {}
401-
pub enum fpos_t {}
400+
pub type FILE;
401+
pub type fpos_t;
402402
}
403403

404404
extern "C" {

src/unix/aix/powerpc64.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use crate::prelude::*;
33

44
// Define lock_data_instrumented as an empty enum
55
extern_ty! {
6-
pub enum lock_data_instrumented {}
6+
pub type lock_data_instrumented;
77
}
88

99
s! {

src/unix/bsd/apple/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ pub type attrgroup_t = u32;
176176
pub type vol_capabilities_set_t = [u32; 4];
177177

178178
extern_ty! {
179-
pub enum timezone {}
179+
pub type timezone;
180180
}
181181

182182
c_enum! {

src/unix/bsd/freebsdlike/dragonfly/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ pub type vm_map_entry_t = *mut vm_map_entry;
4949
pub type pmap = __c_anonymous_pmap;
5050

5151
extern_ty! {
52-
pub enum sem {}
52+
pub type sem;
5353
}
5454

5555
c_enum! {

src/unix/bsd/freebsdlike/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ cfg_if! {
6060
// link.h
6161

6262
extern_ty! {
63-
pub enum timezone {}
63+
pub type timezone;
6464
}
6565

6666
impl siginfo_t {

src/unix/bsd/netbsdlike/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@ pub type sem_t = *mut sem;
1717
pub type key_t = c_long;
1818

1919
extern_ty! {
20-
pub enum timezone {}
21-
pub enum sem {}
20+
pub type timezone;
21+
pub type sem;
2222
}
2323

2424
s! {

0 commit comments

Comments
 (0)