all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Wolfgang Bumiller <w.bumiller@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [PATCH common] tools: use int() on all integer syscall parameters
Date: Tue, 17 May 2022 14:40:03 +0200	[thread overview]
Message-ID: <20220517124003.317063-1-w.bumiller@proxmox.com> (raw)

this should fix an issue where users with custom id mappings
get bad ownership on intermediate directories caused by the
rootuid/gid being the string "100000" in perl instead of the
number 100000...

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
NOTE: I decided to go through them all, not just `fchownat` which was
currently problematic (most likely the issue of [1]).

I hope I got them all right.

This was a frustrating one.

pve needs more rust...

[1] https://forum.proxmox.com/threads/restoring-lxc-from-pbs-fails.108905/#post-471509

 src/PVE/Tools.pm | 91 ++++++++++++++++++++++++++++++++++--------------
 1 file changed, 65 insertions(+), 26 deletions(-)

diff --git a/src/PVE/Tools.pm b/src/PVE/Tools.pm
index dac0a2b..84cb425 100644
--- a/src/PVE/Tools.pm
+++ b/src/PVE/Tools.pm
@@ -1467,32 +1467,39 @@ sub parse_host_and_port {
 
 sub setresuid($$$) {
     my ($ruid, $euid, $suid) = @_;
-    return 0 == syscall(PVE::Syscall::setresuid, $ruid, $euid, $suid);
+    return 0 == syscall(PVE::Syscall::setresuid, int($ruid), int($euid), int($suid));
 }
 
 sub unshare($) {
     my ($flags) = @_;
-    return 0 == syscall(PVE::Syscall::unshare, $flags);
+    return 0 == syscall(PVE::Syscall::unshare, int($flags));
 }
 
 sub setns($$) {
     my ($fileno, $nstype) = @_;
-    return 0 == syscall(PVE::Syscall::setns, $fileno, $nstype);
+    return 0 == syscall(PVE::Syscall::setns, int($fileno), int($nstype));
 }
 
 sub syncfs($) {
     my ($fileno) = @_;
-    return 0 == syscall(PVE::Syscall::syncfs, $fileno);
+    return 0 == syscall(PVE::Syscall::syncfs, int($fileno));
 }
 
 sub fsync($) {
     my ($fileno) = @_;
-    return 0 == syscall(PVE::Syscall::fsync, $fileno);
+    return 0 == syscall(PVE::Syscall::fsync, int($fileno));
 }
 
 sub renameat2($$$$$) {
     my ($olddirfd, $oldpath, $newdirfd, $newpath, $flags) = @_;
-    return 0 == syscall(PVE::Syscall::renameat2, $olddirfd, $oldpath, $newdirfd, $newpath, $flags);
+    return 0 == syscall(
+	PVE::Syscall::renameat2,
+	int($olddirfd),
+	$oldpath,
+	int($newdirfd),
+	$newpath,
+	int($flags),
+    );
 }
 
 sub sync_mountpoint {
@@ -1651,7 +1658,11 @@ sub validate_ssh_public_keys {
 
 sub openat($$$;$) {
     my ($dirfd, $pathname, $flags, $mode) = @_;
-    my $fd = syscall(PVE::Syscall::openat, $dirfd, $pathname, $flags, $mode//0);
+    $dirfd = int($dirfd);
+    $flags = int($flags);
+    $mode = int($mode // 0);
+
+    my $fd = syscall(PVE::Syscall::openat, $dirfd, $pathname, $flags, $mode);
     return undef if $fd < 0;
     # sysopen() doesn't deal with numeric file descriptors apparently
     # so we need to convert to a mode string for IO::Handle->new_from_fd
@@ -1666,12 +1677,19 @@ sub openat($$$;$) {
 
 sub mkdirat($$$) {
     my ($dirfd, $name, $mode) = @_;
-    return syscall(PVE::Syscall::mkdirat, $dirfd, $name, $mode) == 0;
+    return syscall(PVE::Syscall::mkdirat, int($dirfd), $name, int($mode)) == 0;
 }
 
 sub fchownat($$$$$) {
     my ($dirfd, $pathname, $owner, $group, $flags) = @_;
-    return syscall(PVE::Syscall::fchownat, $dirfd, $pathname, $owner, $group, $flags) == 0;
+    return syscall(
+	PVE::Syscall::fchownat,
+	int($dirfd),
+	$pathname,
+	int($owner),
+	int($group),
+	int($flags),
+    ) == 0;
 }
 
 my $salt_starter = time();
@@ -1801,9 +1819,9 @@ sub open_tree($$$) {
     my ($dfd, $pathname, $flags) = @_;
     return PVE::Syscall::file_handle_result(syscall(
 	&PVE::Syscall::open_tree,
-	$dfd,
+	int($dfd),
 	$pathname,
-	$flags,
+	int($flags),
     ));
 }
 
@@ -1811,26 +1829,26 @@ sub move_mount($$$$$) {
     my ($from_dirfd, $from_pathname, $to_dirfd, $to_pathname, $flags) = @_;
     return 0 == syscall(
 	&PVE::Syscall::move_mount,
-	$from_dirfd,
+	int($from_dirfd),
 	$from_pathname,
-	$to_dirfd,
+	int($to_dirfd),
 	$to_pathname,
-	$flags,
+	int($flags),
     );
 }
 
 sub fsopen($$) {
     my ($fsname, $flags) = @_;
-    return PVE::Syscall::file_handle_result(syscall(&PVE::Syscall::fsopen, $fsname, $flags));
+    return PVE::Syscall::file_handle_result(syscall(&PVE::Syscall::fsopen, $fsname, int($flags)));
 }
 
 sub fsmount($$$) {
     my ($fd, $flags, $mount_attrs) = @_;
     return PVE::Syscall::file_handle_result(syscall(
 	&PVE::Syscall::fsmount,
-	$fd,
-	$flags,
-	$mount_attrs,
+	int($fd),
+	int($flags),
+	int($mount_attrs),
     ));
 }
 
@@ -1838,15 +1856,22 @@ sub fspick($$$) {
     my ($dirfd, $pathname, $flags) = @_;
     return PVE::Syscall::file_handle_result(syscall(
 	&PVE::Syscall::fspick,
-	$dirfd,
+	int($dirfd),
 	$pathname,
-	$flags,
+	int($flags),
     ));
 }
 
 sub fsconfig($$$$$) {
     my ($fd, $command, $key, $value, $aux) = @_;
-    return 0 == syscall(&PVE::Syscall::fsconfig, $fd, $command, $key, $value, $aux);
+    return 0 == syscall(
+	&PVE::Syscall::fsconfig,
+	int($fd),
+	int($command),
+	$key,
+	$value,
+	int($aux),
+    );
 }
 
 # "raw" mount, old api, not for generic use (as it does not invoke any helpers).
@@ -1858,7 +1883,7 @@ sub mount($$$$$) {
 	$source,
 	$target,
 	$filesystemtype,
-	$mountflags,
+	int($mountflags),
 	$data,
     );
 }
@@ -1872,9 +1897,9 @@ sub getxattr($$;$) {
 
     my $xattr_size = -1; # the actual size of the xattr, can be zero
     if (defined(my $fd = fileno($path_or_handle))) {
-	$xattr_size = syscall(&PVE::Syscall::fgetxattr, $fd, $name, $buf, $size);
+	$xattr_size = syscall(&PVE::Syscall::fgetxattr, $fd, $name, $buf, int($size));
     } else {
-	$xattr_size = syscall(&PVE::Syscall::getxattr, $path_or_handle, $name, $buf, $size);
+	$xattr_size = syscall(&PVE::Syscall::getxattr, $path_or_handle, $name, $buf, int($size));
     }
     if ($xattr_size < 0) {
 	return undef;
@@ -1889,9 +1914,23 @@ sub setxattr($$$;$) {
     my $size = length($value); # NOTE: seems to get correct length also for wide-characters in text..
 
     if (defined(my $fd = fileno($path_or_handle))) {
-	return 0 == syscall(&PVE::Syscall::fsetxattr, $fd, $name, $value, $size, $flags // 0);
+	return 0 == syscall(
+	    &PVE::Syscall::fsetxattr,
+	    $fd,
+	    $name,
+	    $value,
+	    int($size),
+	    int($flags // 0),
+	);
     } else {
-	return 0 == syscall(&PVE::Syscall::setxattr, $path_or_handle, $name, $value, $size, $flags // 0);
+	return 0 == syscall(
+	    &PVE::Syscall::setxattr,
+	    $path_or_handle,
+	    $name,
+	    $value,
+	    int($size),
+	    int($flags // 0),
+	);
     }
 }
 
-- 
2.30.2





             reply	other threads:[~2022-05-17 12:40 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-17 12:40 Wolfgang Bumiller [this message]
2022-05-20 13:16 ` [pve-devel] applied: " Thomas Lamprecht

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220517124003.317063-1-w.bumiller@proxmox.com \
    --to=w.bumiller@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal