pop-os / system76-firmware

System76 Firmware Tool and Daemon
GNU General Public License v3.0
74 stars 28 forks source link

Build failure, "literal out of range for i32" #22

Open PureTryOut opened 4 years ago

PureTryOut commented 4 years ago

Distribution: Alpine Linux

Application Version: 1.0.5 and latest Git master, commit 699fd04d9d4971dc2e199c08c143f9aefae8174d

Issue/Bug Description: Build failure

   Compiling system76-firmware v1.0.5 (/home/builder/aports/testing/system76-firmware/src/system76-firmware-699fd04d9d4971dc2e199c08c143f9aefae8174d)
error: literal out of range for i32
  --> src/me.rs:62:41
   |
62 |         if unsafe { libc::ioctl(mei_fd, 0xc0104801, uuid_bytes.as_mut_ptr()) } != 0 {
   |                                         ^^^^^^^^^^
   |
   = note: `#[deny(overflowing_literals)]` on by default
   = note: the literal `0xc0104801` (decimal `3222292481`) does not fit into an `i32` and will become `-1072674815i32`
   = help: consider using `u32` instead

error: aborting due to previous error

error: Could not compile `system76-firmware`.
jvvv commented 1 year ago

The cause of this issue is that the ioctl declaration in musl-libc is:

int ioctl(int, int, ...);

This conforms to POSIX, but is problematic because glibc and the linux kernel use this:

int ioctl(int, unsigned long, ...);

I was getting started on an Alpine Linux aport for this and some other System76 software when I ran into this same error. I am looking into a couple of approachs for work arounds. I will update here when I am get close to submitting a PR.

There is also another build failure in src/bin/cli.rs and daemon/src/main.rs regarding the iopl(2) system call. I will be looking in this as well. I think that this one is not a type mismatch, though.

jvvv commented 1 year ago

For the ioctl, a cast to u32:

From aa65cf996b276611ac994233043b6a53c7a927d7 Mon Sep 17 00:00:00 2001
From: John Vogel <jvogel4@stny.rr.com>
Date: Wed, 15 Mar 2023 10:16:07 -0400
Subject: [PATCH] src/me.rs: cast literal to u32

---
 src/me.rs | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/me.rs b/src/me.rs
index 8d5f5ae..559b3ed 100644
--- a/src/me.rs
+++ b/src/me.rs
@@ -5,6 +5,7 @@ use std::io::{Read, Write};
 use std::os::unix::io::AsRawFd;
 use std::path::Path;
 use uuid::Uuid;
+use std::convert::TryInto;

 use crate::err_str;

@@ -59,7 +60,7 @@ pub fn me() -> Result<Option<String>, String> {
             uuid_bytes[8..].copy_from_slice(bytes);
         }

-        if unsafe { libc::ioctl(mei_fd, 0xc0104801, uuid_bytes.as_mut_ptr()) } != 0 {
+        if unsafe { libc::ioctl(mei_fd, (0xc0104801 as u32).try_into().unwrap(), uuid_bytes.as_mut_ptr()) } != 0 {
            return Err(format!(
                "failed to send MEI UUID: {}",
                io::Error::last_os_error()
-- 
2.40.0

looks like it will be ok, though a bit murky. The sign bit shouldn't be lost; it will end up being cast to unsigned long anyways in the c library ioctl function when it calls syscall. Another option might be to use to nix crate to pull in a different ioctl or to hand craft an ioctl function. Both these seem heavy handed to me.

The iopl issue is uglier, because it seems it is not even being pulled in for musl targets in the libc crate.

jvvv commented 1 year ago

I have decided to backed off from this issue until rust libc more fully supports musl libc.

koltenpearson commented 10 months ago

I was able to get it to compile with alpine linux with these changes

diff --git a/daemon/src/main.rs b/daemon/src/main.rs
index b3a97bb..301bd7f 100644
--- a/daemon/src/main.rs
+++ b/daemon/src/main.rs
@@ -18,7 +18,7 @@ fn daemon() -> Result<(), String> {
     }

     // Get I/O Permission
-    if unsafe { libc::iopl(3) } < 0 {
+    if unsafe { libc::syscall(libc::SYS_iopl,3) } < 0 {
         return Err(format!(
             "failed to get I/O permission: {}",
             io::Error::last_os_error()
diff --git a/src/bin/cli.rs b/src/bin/cli.rs
index 0e9d70c..e21a72d 100644
--- a/src/bin/cli.rs
+++ b/src/bin/cli.rs
@@ -32,7 +32,7 @@ fn tool() -> Result<(), String> {
     }

     // Get I/O Permission
-    if unsafe { libc::iopl(3) } < 0 {
+    if unsafe { libc::syscall(libc::SYS_iopl, 3) } < 0 {
         return Err(format!(
             "failed to get I/O permission: {}",
             io::Error::last_os_error()
diff --git a/src/me.rs b/src/me.rs
index 8d5f5ae..559b3ed 100644
--- a/src/me.rs
+++ b/src/me.rs
@@ -5,6 +5,7 @@ use std::io::{Read, Write};
 use std::os::unix::io::AsRawFd;
 use std::path::Path;
 use uuid::Uuid;
+use std::convert::TryInto;

 use crate::err_str;

@@ -59,7 +60,7 @@ pub fn me() -> Result<Option<String>, String> {
             uuid_bytes[8..].copy_from_slice(bytes);
         }

-        if unsafe { libc::ioctl(mei_fd, 0xc0104801, uuid_bytes.as_mut_ptr()) } != 0 {
+        if unsafe { libc::ioctl(mei_fd, (0xc0104801 as u32).try_into().unwrap(), uuid_bytes.as_mut_ptr()) } != 0 {
            return Err(format!(
                "failed to send MEI UUID: {}",
                io::Error::last_os_error()

iopl() is just a thin wrapper around the syscall in any case