Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

utsame struct is incorrectly defined on FreebSD #1190

Closed
inferiorhumanorgans opened this issue Jan 2, 2019 · 4 comments
Closed

utsame struct is incorrectly defined on FreebSD #1190

inferiorhumanorgans opened this issue Jan 2, 2019 · 4 comments

Comments

@inferiorhumanorgans
Copy link
Contributor

On FreeBSD utsname struct is defined with 32 character-wide arrays for the kernel interface and 256-wide arrays for the userland interface. The userland interface is a small inline function in sys/utsname.h that calls the kernel interface and explicitly requests longer fields. Unfortunately rust is calling the kernel interface directly and only getting arrays that are 32 characters in length. As a result each field from the C struct ends up in the first field in the rust struct, and the other rust fields remain empty.

For example:

extern crate libc;

use std::ffi::CStr;
use std::process::Command;

use libc::{c_char, c_int};

#[repr(C)]
struct small_utsname {
  pub sysname:  [c_char; 32],
  pub nodename: [c_char; 32],
  pub release:  [c_char; 32],
  pub version:  [c_char; 32],
  pub machine:  [c_char; 32],
}

fn to_cstr(buf: &[c_char]) -> &CStr {
  unsafe { CStr::from_ptr(buf.as_ptr()) }
}


#[link(name = "c")]
extern { fn uname(buf: *mut small_utsname) -> c_int; }

fn main() {
   const UNAME_PATH : &str = "/usr/bin/uname";

   let uname_1_s = Command::new(UNAME_PATH).arg("-s").output().unwrap();
   let uname_1_r = Command::new(UNAME_PATH).arg("-r").output().unwrap();

   let inferred_system = String::from_utf8_lossy(&uname_1_s.stdout);
   let inferred_release = String::from_utf8_lossy(&uname_1_r.stdout);

   {
     println!("width = 32");
     let mut uts : small_utsname = unsafe { std::mem::zeroed() };;
     unsafe { uname(&mut uts) };

     let sysname : String = to_cstr(&uts.sysname[..]).to_string_lossy().into_owned();
     let release : String = to_cstr(&uts.release[..]).to_string_lossy().into_owned();

     debug_assert_eq!(sysname, inferred_system.trim());
     debug_assert_eq!(release, inferred_release.trim());
   }

   {
     println!("width = 256");
     let mut uts : libc::utsname = unsafe { std::mem::zeroed() };;
     unsafe { libc::uname(&mut uts) };

     let sysname : String = to_cstr(&uts.sysname[..]).to_string_lossy().into_owned();
     let release : String = to_cstr(&uts.release[..]).to_string_lossy().into_owned();

     debug_assert_eq!(sysname, inferred_system.trim());
     debug_assert_eq!(release, inferred_release.trim());
   }
}
@inferiorhumanorgans inferiorhumanorgans changed the title uname struct is incorrect on FreebSD utsame struct is incorrectly defined on FreebSD Jan 2, 2019
@gnzlbg
Copy link
Contributor

gnzlbg commented Jan 2, 2019

The appropriate header is here: https://github.com/freebsd/freebsd/blob/1d6e4247415d264485ee94b59fdbc12e0c566fd0/sys/sys/utsname.h#L49

The length of the arrays for the user-land interface is actually configurable.

Unfortunately rust is calling the kernel interface directly

Indeed, the length of those arrays should be the length that the kernel interface expects.

I wondered why the tests aren't catching this error, and it appears that we are just skipping the tests for uname:

"uname" if freebsd => true,

Instead of skipping the tests, the right way to test this is to define _KERNEL when loading the appropriate header.

@inferiorhumanorgans
Copy link
Contributor Author

inferiorhumanorgans commented Jan 2, 2019

It looks like the test is skipped because uname is defined inline in sys/utsname.h so the assumption is that there's no uname function in libc. In reality it looks like there's (also) a uname function in libc that presumably runs with the length set to 32. Basically it works accidentally.

$ uname -a
FreeBSD devbox 12.0-RELEASE FreeBSD 12.0-RELEASE r341666 GENERIC  amd64
$ readelf -s /lib/libc.so.7  | grep uname
  2190: 00000000000ab190    21 FUNC    GLOBAL DEFAULT   12 uname@@FBSD_1.0 (2)
  2856: 000000000013b5b0   759 FUNC    GLOBAL DEFAULT   12 __xuname@@FBSD_1.0 (2)

@asomers
Copy link
Contributor

asomers commented Jan 2, 2019

libc's uname is at https://svnweb.freebsd.org/base/head/lib/libc/gen/uname.c?view=markup . The inline function was added in r74729 (back in 2001!) and is what all new programs should be using. The version in libc is for binary compatibility with very old programs. All Rust programs should be using the inline version. Unfortunately, that probably means that libc needs to deprecate uname and only expose __xuname.

See Also:
https://svnweb.freebsd.org/base/head/lib/libc/gen/uname.c?revision=202661&view=markup
https://svnweb.freebsd.org/base?view=revision&revision=74729

@inferiorhumanorgans
Copy link
Contributor Author

Well if the goal is to replicate libc I'd say the ideal should be to wrap __xuname instead of exposing it directly.

asomers added a commit to asomers/libc that referenced this issue Jan 3, 2019
On FreeBSD, uname is an inline function.  The uname that is present in
libc.so is for FreeBSD 1.0 compatibility.  It expects a buffer of a
different size.

Fixes rust-lang#1190
Reported-by: Alex Zepeda
bors added a commit that referenced this issue Jan 3, 2019
Fix uname on FreeBSD

On FreeBSD, uname is an inline function.  The uname that is present in
libc.so is for FreeBSD 1.0 compatibility.  It expects a buffer of a
different size.

Fixes #1190
Reported-by: Alex Zepeda
bors added a commit that referenced this issue Jan 3, 2019
Fix uname on FreeBSD

On FreeBSD, uname is an inline function.  The uname that is present in
libc.so is for FreeBSD 1.0 compatibility.  It expects a buffer of a
different size.

Fixes #1190
Reported-by: Alex Zepeda
@bors bors closed this as completed in #1196 Jan 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants