Support negative --{uid,gid}-offset

Fixes #142

3 files changed, 69 insertions, 22 deletions

diff --git a/ChangeLog b/ChangeLog
index c257754..eb0845e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2023-11-09  Martin Pärtel <martin dot partel at gmail dot com>
+
+	* Support negative --{uid,gid}-offset (issue #142)
+
 2023-09-22  Martin Pärtel <martin dot partel at gmail dot com>
 
 	* Support `mount -t bindfs` on MacOS (PR #140, thanks @Kentzo)
diff --git a/src/bindfs.c b/src/bindfs.c
index 3ef8bf7..dd32800 100644
--- a/src/bindfs.c
+++ b/src/bindfs.c
@@ -35,6 +35,7 @@
 #include <stddef.h>
 #include <stdio.h>
 #include <stdbool.h>
+#include <stdint.h>
 #include <string.h>
 #include <ctype.h>
 #ifdef HAVE_SYS_TYPES_H
@@ -116,8 +117,8 @@ int flock(int fd, int operation);
 
 /* We pessimistically assume signed uid_t and gid_t in our overflow checks,
    mostly because supporting both cases would require a bunch more code. */
-static const uid_t UID_T_MAX = ((1LL << (sizeof(uid_t)*8-1)) - 1);
-static const gid_t GID_T_MAX = ((1LL << (sizeof(gid_t)*8-1)) - 1);
+static const int64_t UID_T_MAX = ((1LL << (sizeof(uid_t)*8-1)) - 1);
+static const int64_t GID_T_MAX = ((1LL << (sizeof(gid_t)*8-1)) - 1);
 static const int UID_GID_OVERFLOW_ERRNO = EIO;
 
 /* SETTINGS */
@@ -213,8 +214,8 @@ static struct Settings {
     bool direct_io;
 #endif
 
-    uid_t uid_offset;
-    gid_t gid_offset;
+    int64_t uid_offset;
+    int64_t gid_offset;
 
 } settings;
 
@@ -635,39 +636,47 @@ static int delete_file(const char *path, int (*target_delete_func)(const char *)
 }
 
 static int apply_uid_offset(uid_t *uid) {
-    if (*uid > UID_T_MAX - settings.uid_offset) {
-        DPRINTF("UID %lld overflowed while applying offset", (long long)*uid);
+    uid_t result = *uid + (uid_t)settings.uid_offset;
+    if (0 <= result && result <= UID_T_MAX) {
+        *uid = result;
+        return 1;
+    } else {
+        DPRINTF("UID %ld overflowed while applying offset", (int64_t)*uid);
         return 0;
     }
-    *uid += settings.uid_offset;
-    return 1;
 }
 
 static int apply_gid_offset(gid_t *gid) {
-    if (*gid > GID_T_MAX - settings.gid_offset) {
-        DPRINTF("GID %lld overflowed while applying offset", (long long)*gid);
+    gid_t result = *gid + (gid_t)settings.gid_offset;
+    if (0 <= result && result <= GID_T_MAX) {
+        *gid = result;
+        return 1;
+    } else {
+        DPRINTF("GID %ld overflowed while applying offset", (int64_t)*gid);
         return 0;
     }
-    *gid += settings.gid_offset;
-    return 1;
 }
 
 static int unapply_uid_offset(uid_t *uid) {
-    if (*uid < settings.uid_offset) {
-        DPRINTF("UID %lld underflowed while unapplying offset", (long long)*uid);
+    uid_t result = *uid - (uid_t)settings.uid_offset;
+    if (0 <= result && result <= UID_T_MAX) {
+        *uid = result;
+        return 1;
+    } else {
+        DPRINTF("UID %ld underflowed while applying offset", (int64_t)*uid);
         return 0;
     }
-    *uid -= settings.uid_offset;
-    return 1;
 }
 
 static int unapply_gid_offset(gid_t *gid) {
-  if (*gid < settings.gid_offset) {
-        DPRINTF("GID %lld underflowed while unapplying offset", (long long)*gid);
+    gid_t result = *gid - (gid_t)settings.gid_offset;
+    if (0 <= result && result <= GID_T_MAX) {
+        *gid = result;
+        return 1;
+    } else {
+        DPRINTF("GID %ld underflowed while applying offset", (int64_t)*gid);
         return 0;
     }
-    *gid -= settings.gid_offset;
-    return 1;
 }
 
 #ifdef __linux__
@@ -2680,7 +2689,7 @@ int main(int argc, char *argv[])
             return 1;
         }
         char* endptr = od.uid_offset;
-        settings.uid_offset = strtoul(od.uid_offset, &endptr, 10);
+        settings.uid_offset = strtoll(od.uid_offset, &endptr, 10);
         if (*endptr != '\0') {
             fprintf(stderr, "Error: Value of --uid-offset must be an integer.\n");
             return 1;
@@ -2697,7 +2706,7 @@ int main(int argc, char *argv[])
             return 1;
         }
         char* endptr = od.gid_offset;
-        settings.gid_offset = strtoul(od.gid_offset, &endptr, 10);
+        settings.gid_offset = strtoll(od.gid_offset, &endptr, 10);
         if (*endptr != '\0') {
             fprintf(stderr, "Error: Value of --gid-offset must be an integer.\n");
             return 1;
diff --git a/tests/test_bindfs.rb b/tests/test_bindfs.rb
index 031529b..5adda52 100755
--- a/tests/test_bindfs.rb
+++ b/tests/test_bindfs.rb
@@ -68,6 +68,9 @@ $nobody_gid = nobody_gid = Etc.getpwnam('nobody').gid
 $nobody_group = nobody_group = Etc.getgrgid(nobody_gid).name
 $root_group = root_group = Etc.getgrgid(0).name
 
+$user_1k = user_1k = begin Etc.getpwuid(1000).name; rescue; nil; end
+$user_1k_group = user_1k_group = begin Etc.getgrgid(Etc.getpwuid(1000).gid).name; rescue; nil; end
+
 $tests_dir = File.realpath('.')
 
 
@@ -471,8 +474,25 @@ root_testenv("--uid-offset=2 --gid-offset=20", :title => "file creation with --u
     assert { File.stat('mnt/file').gid == File.stat('src/file').gid + 20 }
 end
 
+# This test requires user 1k to actually exist so we can sudo to it
+if user_1k
+    root_testenv("--uid-offset=-2 --gid-offset=-20", :title => "file creation with negative --uid-offset and --gid-offset") do
+        chown(user_1k, user_1k_group, 'src')
+        chmod(0777, 'src')
+        `sudo -u #{user_1k} -g #{user_1k_group} touch mnt/file`
+
+        assert { File.stat('src/file').uid == 1002 }
+        assert { File.stat('mnt/file').uid == 1000 }
+        assert { File.stat('mnt/file').gid == File.stat('src/file').gid - 20 }
+    end
+end
+
 root_testenv("--uid-offset=2 --gid-offset=20", :title => "chown/chgrp with --uid-offset and --gid-offset") do
     touch('src/file')
+    # Avoid I/O error due to uid and gid = 0 not being mappable with negative offsets
+    chown(1000, 1000, 'src')
+    chown(1000, 1000, 'src/file')
+
     chown(6, 25, 'mnt/file')
 
     assert { File.stat('src/file').uid == 4 }
@@ -481,6 +501,20 @@ root_testenv("--uid-offset=2 --gid-offset=20", :title => "chown/chgrp with --uid
     assert { File.stat('mnt/file').gid == 25 }
 end
 
+root_testenv("--uid-offset=-2 --gid-offset=-20", :title => "chown/chgrp with negative --uid-offset and --gid-offset") do
+    touch('src/file')
+    # Avoid I/O error due to uid and gid = 0 not being mappable with negative offsets
+    chown(1000, 1000, 'src')
+    chown(1000, 1000, 'src/file')
+
+    chown(6, 25, 'mnt/file')
+
+    assert { File.stat('src/file').uid == 8 }
+    assert { File.stat('src/file').gid == 45 }
+    assert { File.stat('mnt/file').uid == 6 }
+    assert { File.stat('mnt/file').gid == 25 }
+end
+
 testenv("", :title => "preserves inode numbers") do
     touch('src/file')
     mkdir('src/dir')