[pbs-devel] [PATCH proxmox v2 1/2] sys: fs: set CLOEXEC when creating temp files

[pbs-devel] [PATCH proxmox v2 1/2] sys: fs: set CLOEXEC when creating temp files

[Dominik Csapak]

In general we want all open files to have set CLOEXEC since our
reloading mechanism can basically fork at any moment and we don't want
newer daemons to carry around old file descriptors, especially lock
files.

Since `make_tmp_file` is called by many things (e.g. open_file_locked,
logrotate, rrd), set O_CLOEXEC with mkostemp.

This fixes issues with leftover file descriptors e.g. tape backups not
working because of lingering locks after a reload, or having deleted
rrd files open.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v1:
* introduce mkostemp helper which is similar to nix's mkstemp helper
  (the code is a copy of mkstemp aside from the call to libcmkostemp +
  the oflag handling)

  I did it this way, since we may be able to upstream this, have
  to look more closer at this though.

 proxmox-sys/src/fs/file.rs | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/proxmox-sys/src/fs/file.rs b/proxmox-sys/src/fs/file.rs
index fbfc0b58..74b9e74e 100644
--- a/proxmox-sys/src/fs/file.rs
+++ b/proxmox-sys/src/fs/file.rs
@@ -116,6 +116,29 @@ pub fn file_read_firstline<P: AsRef<Path>>(path: P) -> Result<String, Error> {
     read_firstline(path).map_err(|err| format_err!("unable to read {path:?} - {err}"))
 }
 
+#[inline]
+/// Creates a tmpfile like [`nix::unistd::mkstemp`], but with [`nix::fctnl::Oflag`] set.
+///
+/// Note that some flags are masked out since they can produce an error, see mkostemp(2) for details.
+// code is mostly copied from nix mkstemp
+fn mkostemp<P: ?Sized + NixPath>(
+    template: &P,
+    oflag: OFlag,
+) -> nix::Result<(std::os::fd::RawFd, PathBuf)> {
+    use std::os::unix::ffi::OsStringExt;
+    let mut path = template.with_nix_path(|path| path.to_bytes_with_nul().to_owned())?;
+    let p = path.as_mut_ptr().cast();
+
+    let flags = OFlag::intersection(OFlag::O_APPEND | OFlag::O_CLOEXEC | OFlag::O_SYNC, oflag);
+
+    let fd = unsafe { libc::mkostemp(p, flags.bits()) };
+    let last = path.pop(); // drop the trailing nul
+    debug_assert!(last == Some(b'\0'));
+    let pathname = std::ffi::OsString::from_vec(path);
+    Errno::result(fd)?;
+    Ok((fd, PathBuf::from(pathname)))
+}
+
 /// Takes a Path and CreateOptions, creates a tmpfile from it and returns
 /// a RawFd and PathBuf for it
 pub fn make_tmp_file<P: AsRef<Path>>(
@@ -127,7 +150,7 @@ pub fn make_tmp_file<P: AsRef<Path>>(
     // use mkstemp here, because it works with different processes, threads, even tokio tasks
     let mut template = path.to_owned();
     template.set_extension("tmp_XXXXXX");
-    let (mut file, tmp_path) = match unistd::mkstemp(&template) {
+    let (mut file, tmp_path) = match mkostemp(&template, OFlag::O_CLOEXEC) {
         Ok((fd, path)) => (unsafe { File::from_raw_fd(fd) }, path),
         Err(err) => bail!("mkstemp {:?} failed: {}", template, err),
     };
-- 
2.39.5



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH proxmox v2 2/2] sys: open directories with O_CLOEXEC

[Dominik Csapak]

so they don't linger around in case of a daemon reload.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
new in v2
 proxmox-sys/src/fd.rs     |  2 +-
 proxmox-sys/src/fs/dir.rs | 15 +++++++++------
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/proxmox-sys/src/fd.rs b/proxmox-sys/src/fd.rs
index 8d85bd2e..386e4222 100644
--- a/proxmox-sys/src/fd.rs
+++ b/proxmox-sys/src/fd.rs
@@ -24,7 +24,7 @@ pub fn change_cloexec(fd: RawFd, on: bool) -> Result<(), anyhow::Error> {
 }
 
 pub(crate) fn cwd() -> Result<OwnedFd, nix::Error> {
-    open(".", OFlag::O_DIRECTORY, stat::Mode::empty())
+    open(".", crate::fs::DIR_FLAGS, stat::Mode::empty())
 }
 
 pub fn open<P>(path: &P, oflag: OFlag, mode: Mode) -> Result<OwnedFd, nix::Error>
diff --git a/proxmox-sys/src/fs/dir.rs b/proxmox-sys/src/fs/dir.rs
index c903ab87..a093ed99 100644
--- a/proxmox-sys/src/fs/dir.rs
+++ b/proxmox-sys/src/fs/dir.rs
@@ -14,6 +14,9 @@ use proxmox_lang::try_block;
 
 use crate::fs::{fchown, CreateOptions};
 
+/// The default [`OFlag`] we want to use when opening directories.
+pub(crate) const DIR_FLAGS: OFlag = OFlag::O_DIRECTORY.union(OFlag::O_CLOEXEC);
+
 /// Creates directory at the provided path with specified ownership.
 ///
 /// Errors if the directory already exists.
@@ -66,7 +69,7 @@ pub fn ensure_dir_exists<P: AsRef<Path>>(
         Err(err) => bail!("unable to create directory {path:?} - {err}",),
     }
 
-    let fd = nix::fcntl::open(path, OFlag::O_DIRECTORY, stat::Mode::empty())
+    let fd = nix::fcntl::open(path, DIR_FLAGS, stat::Mode::empty())
         .map(|fd| unsafe { OwnedFd::from_raw_fd(fd) })
         .map_err(|err| format_err!("unable to open created directory {path:?} - {err}"))?;
     // umask defaults to 022 so make sure the mode is fully honowed:
@@ -120,7 +123,7 @@ fn create_path_do(
         Some(Component::Prefix(_)) => bail!("illegal prefix path component encountered"),
         Some(Component::RootDir) => {
             let _ = iter.next();
-            crate::fd::open(c"/", OFlag::O_DIRECTORY, stat::Mode::empty())?
+            crate::fd::open(c"/", DIR_FLAGS, stat::Mode::empty())?
         }
         Some(Component::CurDir) => {
             let _ = iter.next();
@@ -128,7 +131,7 @@ fn create_path_do(
         }
         Some(Component::ParentDir) => {
             let _ = iter.next();
-            crate::fd::open(c"..", OFlag::O_DIRECTORY, stat::Mode::empty())?
+            crate::fd::open(c"..", DIR_FLAGS, stat::Mode::empty())?
         }
         Some(Component::Normal(_)) => {
             // simply do not advance the iterator, heavy lifting happens in create_path_at_do()
@@ -154,7 +157,7 @@ fn create_path_at_do(
             None => return Ok(created),
 
             Some(Component::ParentDir) => {
-                at = crate::fd::openat(&at, c"..", OFlag::O_DIRECTORY, stat::Mode::empty())?;
+                at = crate::fd::openat(&at, c"..", DIR_FLAGS, stat::Mode::empty())?;
             }
 
             Some(Component::Normal(path)) => {
@@ -175,7 +178,7 @@ fn create_path_at_do(
                     Err(e) => return Err(e.into()),
                     Ok(_) => true,
                 };
-                at = crate::fd::openat(&at, path, OFlag::O_DIRECTORY, stat::Mode::empty())?;
+                at = crate::fd::openat(&at, path, DIR_FLAGS, stat::Mode::empty())?;
 
                 if let (true, Some(opts)) = (created, opts) {
                     if opts.owner.is_some() || opts.group.is_some() {
@@ -222,7 +225,7 @@ pub fn make_tmp_dir<P: AsRef<Path>>(
 
     if let Some(options) = options {
         if let Err(err) = try_block!({
-            let mut fd = crate::fd::open(&path, OFlag::O_DIRECTORY, stat::Mode::empty())?;
+            let mut fd = crate::fd::open(&path, DIR_FLAGS, stat::Mode::empty())?;
             options.apply_to(&mut fd, &path)?;
             Ok::<(), Error>(())
         }) {
-- 
2.39.5



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH proxmox v2 2/2] sys: open directories with O_CLOEXEC

[Fabian Grünbichler]

one small nit inline, otherwise:

Reviewed-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>

On November 29, 2024 3:28 pm, Dominik Csapak wrote:
> so they don't linger around in case of a daemon reload.
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> new in v2
>  proxmox-sys/src/fd.rs     |  2 +-
>  proxmox-sys/src/fs/dir.rs | 15 +++++++++------
>  2 files changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/proxmox-sys/src/fd.rs b/proxmox-sys/src/fd.rs
> index 8d85bd2e..386e4222 100644
> --- a/proxmox-sys/src/fd.rs
> +++ b/proxmox-sys/src/fd.rs
> @@ -24,7 +24,7 @@ pub fn change_cloexec(fd: RawFd, on: bool) -> Result<(), anyhow::Error> {
>  }
>  
>  pub(crate) fn cwd() -> Result<OwnedFd, nix::Error> {
> -    open(".", OFlag::O_DIRECTORY, stat::Mode::empty())
> +    open(".", crate::fs::DIR_FLAGS, stat::Mode::empty())
>  }
>  
>  pub fn open<P>(path: &P, oflag: OFlag, mode: Mode) -> Result<OwnedFd, nix::Error>
> diff --git a/proxmox-sys/src/fs/dir.rs b/proxmox-sys/src/fs/dir.rs
> index c903ab87..a093ed99 100644
> --- a/proxmox-sys/src/fs/dir.rs
> +++ b/proxmox-sys/src/fs/dir.rs
> @@ -14,6 +14,9 @@ use proxmox_lang::try_block;
>  
>  use crate::fs::{fchown, CreateOptions};
>  
> +/// The default [`OFlag`] we want to use when opening directories.
> +pub(crate) const DIR_FLAGS: OFlag = OFlag::O_DIRECTORY.union(OFlag::O_CLOEXEC);

nit: I think I'd prefer a plain `|` here (they are the same in the
bitflags crate, which this is under the hood).

> +
>  /// Creates directory at the provided path with specified ownership.
>  ///
>  /// Errors if the directory already exists.
> @@ -66,7 +69,7 @@ pub fn ensure_dir_exists<P: AsRef<Path>>(
>          Err(err) => bail!("unable to create directory {path:?} - {err}",),
>      }
>  
> -    let fd = nix::fcntl::open(path, OFlag::O_DIRECTORY, stat::Mode::empty())
> +    let fd = nix::fcntl::open(path, DIR_FLAGS, stat::Mode::empty())
>          .map(|fd| unsafe { OwnedFd::from_raw_fd(fd) })
>          .map_err(|err| format_err!("unable to open created directory {path:?} - {err}"))?;
>      // umask defaults to 022 so make sure the mode is fully honowed:
> @@ -120,7 +123,7 @@ fn create_path_do(
>          Some(Component::Prefix(_)) => bail!("illegal prefix path component encountered"),
>          Some(Component::RootDir) => {
>              let _ = iter.next();
> -            crate::fd::open(c"/", OFlag::O_DIRECTORY, stat::Mode::empty())?
> +            crate::fd::open(c"/", DIR_FLAGS, stat::Mode::empty())?
>          }
>          Some(Component::CurDir) => {
>              let _ = iter.next();
> @@ -128,7 +131,7 @@ fn create_path_do(
>          }
>          Some(Component::ParentDir) => {
>              let _ = iter.next();
> -            crate::fd::open(c"..", OFlag::O_DIRECTORY, stat::Mode::empty())?
> +            crate::fd::open(c"..", DIR_FLAGS, stat::Mode::empty())?
>          }
>          Some(Component::Normal(_)) => {
>              // simply do not advance the iterator, heavy lifting happens in create_path_at_do()
> @@ -154,7 +157,7 @@ fn create_path_at_do(
>              None => return Ok(created),
>  
>              Some(Component::ParentDir) => {
> -                at = crate::fd::openat(&at, c"..", OFlag::O_DIRECTORY, stat::Mode::empty())?;
> +                at = crate::fd::openat(&at, c"..", DIR_FLAGS, stat::Mode::empty())?;
>              }
>  
>              Some(Component::Normal(path)) => {
> @@ -175,7 +178,7 @@ fn create_path_at_do(
>                      Err(e) => return Err(e.into()),
>                      Ok(_) => true,
>                  };
> -                at = crate::fd::openat(&at, path, OFlag::O_DIRECTORY, stat::Mode::empty())?;
> +                at = crate::fd::openat(&at, path, DIR_FLAGS, stat::Mode::empty())?;
>  
>                  if let (true, Some(opts)) = (created, opts) {
>                      if opts.owner.is_some() || opts.group.is_some() {
> @@ -222,7 +225,7 @@ pub fn make_tmp_dir<P: AsRef<Path>>(
>  
>      if let Some(options) = options {
>          if let Err(err) = try_block!({
> -            let mut fd = crate::fd::open(&path, OFlag::O_DIRECTORY, stat::Mode::empty())?;
> +            let mut fd = crate::fd::open(&path, DIR_FLAGS, stat::Mode::empty())?;
>              options.apply_to(&mut fd, &path)?;
>              Ok::<(), Error>(())
>          }) {
> -- 
> 2.39.5
> 
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
> 
> 
> 


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH proxmox v2 2/2] sys: open directories with O_CLOEXEC

[Dominik Csapak]

On 12/2/24 15:01, Fabian Grünbichler wrote:
> one small nit inline, otherwise:
> 
> Reviewed-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> 
> On November 29, 2024 3:28 pm, Dominik Csapak wrote:
>> so they don't linger around in case of a daemon reload.
>>
>> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
>> ---
>> new in v2
>>   proxmox-sys/src/fd.rs     |  2 +-
>>   proxmox-sys/src/fs/dir.rs | 15 +++++++++------
>>   2 files changed, 10 insertions(+), 7 deletions(-)
>>
>> diff --git a/proxmox-sys/src/fd.rs b/proxmox-sys/src/fd.rs
>> index 8d85bd2e..386e4222 100644
>> --- a/proxmox-sys/src/fd.rs
>> +++ b/proxmox-sys/src/fd.rs
>> @@ -24,7 +24,7 @@ pub fn change_cloexec(fd: RawFd, on: bool) -> Result<(), anyhow::Error> {
>>   }
>>   
>>   pub(crate) fn cwd() -> Result<OwnedFd, nix::Error> {
>> -    open(".", OFlag::O_DIRECTORY, stat::Mode::empty())
>> +    open(".", crate::fs::DIR_FLAGS, stat::Mode::empty())
>>   }
>>   
>>   pub fn open<P>(path: &P, oflag: OFlag, mode: Mode) -> Result<OwnedFd, nix::Error>
>> diff --git a/proxmox-sys/src/fs/dir.rs b/proxmox-sys/src/fs/dir.rs
>> index c903ab87..a093ed99 100644
>> --- a/proxmox-sys/src/fs/dir.rs
>> +++ b/proxmox-sys/src/fs/dir.rs
>> @@ -14,6 +14,9 @@ use proxmox_lang::try_block;
>>   
>>   use crate::fs::{fchown, CreateOptions};
>>   
>> +/// The default [`OFlag`] we want to use when opening directories.
>> +pub(crate) const DIR_FLAGS: OFlag = OFlag::O_DIRECTORY.union(OFlag::O_CLOEXEC);
> 
> nit: I think I'd prefer a plain `|` here (they are the same in the
> bitflags crate, which this is under the hood).
> 

had the same thought at first, but the `BitOr` traits (and i guess all traits) are not
const, so we can't directly to this here.

what would be possible is this:

---
pub(crate) const DIR_FLAGS: OFlag =
     OFlag::from_bits_truncate(OFlag::O_DIRECTORY.bits() | OFlag::O_CLOEXEC.bits());
---

which is IMHO even uglier than using `.union()` ...


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH proxmox v2 2/2] sys: open directories with O_CLOEXEC

[Fabian Grünbichler]

On December 2, 2024 3:55 pm, Dominik Csapak wrote:
> On 12/2/24 15:01, Fabian Grünbichler wrote:
>> one small nit inline, otherwise:
>> 
>> Reviewed-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
>> 
>> On November 29, 2024 3:28 pm, Dominik Csapak wrote:
>>> so they don't linger around in case of a daemon reload.
>>>
>>> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
>>> ---
>>> new in v2
>>>   proxmox-sys/src/fd.rs     |  2 +-
>>>   proxmox-sys/src/fs/dir.rs | 15 +++++++++------
>>>   2 files changed, 10 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/proxmox-sys/src/fd.rs b/proxmox-sys/src/fd.rs
>>> index 8d85bd2e..386e4222 100644
>>> --- a/proxmox-sys/src/fd.rs
>>> +++ b/proxmox-sys/src/fd.rs
>>> @@ -24,7 +24,7 @@ pub fn change_cloexec(fd: RawFd, on: bool) -> Result<(), anyhow::Error> {
>>>   }
>>>   
>>>   pub(crate) fn cwd() -> Result<OwnedFd, nix::Error> {
>>> -    open(".", OFlag::O_DIRECTORY, stat::Mode::empty())
>>> +    open(".", crate::fs::DIR_FLAGS, stat::Mode::empty())
>>>   }
>>>   
>>>   pub fn open<P>(path: &P, oflag: OFlag, mode: Mode) -> Result<OwnedFd, nix::Error>
>>> diff --git a/proxmox-sys/src/fs/dir.rs b/proxmox-sys/src/fs/dir.rs
>>> index c903ab87..a093ed99 100644
>>> --- a/proxmox-sys/src/fs/dir.rs
>>> +++ b/proxmox-sys/src/fs/dir.rs
>>> @@ -14,6 +14,9 @@ use proxmox_lang::try_block;
>>>   
>>>   use crate::fs::{fchown, CreateOptions};
>>>   
>>> +/// The default [`OFlag`] we want to use when opening directories.
>>> +pub(crate) const DIR_FLAGS: OFlag = OFlag::O_DIRECTORY.union(OFlag::O_CLOEXEC);
>> 
>> nit: I think I'd prefer a plain `|` here (they are the same in the
>> bitflags crate, which this is under the hood).
>> 
> 
> had the same thought at first, but the `BitOr` traits (and i guess all traits) are not
> const, so we can't directly to this here.
> 
> what would be possible is this:
> 
> ---
> pub(crate) const DIR_FLAGS: OFlag =
>      OFlag::from_bits_truncate(OFlag::O_DIRECTORY.bits() | OFlag::O_CLOEXEC.bits());
> ---
> 
> which is IMHO even uglier than using `.union()` ...

definitely uglier, and yes, I totally missed that:

https://github.com/rust-lang/rust-project-goals/issues/106
https://github.com/rust-lang/rust/issues/67792

and

https://github.com/bitflags/bitflags/issues/180

the latter which explicitly calls out `.union` and friends being added
to work around this limitation..


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH proxmox v2 1/2] sys: fs: set CLOEXEC when creating temp files

[Fabian Grünbichler]

Reviewed-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>

On November 29, 2024 3:28 pm, Dominik Csapak wrote:
> In general we want all open files to have set CLOEXEC since our
> reloading mechanism can basically fork at any moment and we don't want
> newer daemons to carry around old file descriptors, especially lock
> files.
> 
> Since `make_tmp_file` is called by many things (e.g. open_file_locked,
> logrotate, rrd), set O_CLOEXEC with mkostemp.
> 
> This fixes issues with leftover file descriptors e.g. tape backups not
> working because of lingering locks after a reload, or having deleted
> rrd files open.
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> changes from v1:
> * introduce mkostemp helper which is similar to nix's mkstemp helper
>   (the code is a copy of mkstemp aside from the call to libcmkostemp +
>   the oflag handling)
> 
>   I did it this way, since we may be able to upstream this, have
>   to look more closer at this though.
> 
>  proxmox-sys/src/fs/file.rs | 25 ++++++++++++++++++++++++-
>  1 file changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/proxmox-sys/src/fs/file.rs b/proxmox-sys/src/fs/file.rs
> index fbfc0b58..74b9e74e 100644
> --- a/proxmox-sys/src/fs/file.rs
> +++ b/proxmox-sys/src/fs/file.rs
> @@ -116,6 +116,29 @@ pub fn file_read_firstline<P: AsRef<Path>>(path: P) -> Result<String, Error> {
>      read_firstline(path).map_err(|err| format_err!("unable to read {path:?} - {err}"))
>  }
>  
> +#[inline]
> +/// Creates a tmpfile like [`nix::unistd::mkstemp`], but with [`nix::fctnl::Oflag`] set.
> +///
> +/// Note that some flags are masked out since they can produce an error, see mkostemp(2) for details.
> +// code is mostly copied from nix mkstemp
> +fn mkostemp<P: ?Sized + NixPath>(
> +    template: &P,
> +    oflag: OFlag,
> +) -> nix::Result<(std::os::fd::RawFd, PathBuf)> {
> +    use std::os::unix::ffi::OsStringExt;
> +    let mut path = template.with_nix_path(|path| path.to_bytes_with_nul().to_owned())?;
> +    let p = path.as_mut_ptr().cast();
> +
> +    let flags = OFlag::intersection(OFlag::O_APPEND | OFlag::O_CLOEXEC | OFlag::O_SYNC, oflag);
> +
> +    let fd = unsafe { libc::mkostemp(p, flags.bits()) };
> +    let last = path.pop(); // drop the trailing nul
> +    debug_assert!(last == Some(b'\0'));
> +    let pathname = std::ffi::OsString::from_vec(path);
> +    Errno::result(fd)?;
> +    Ok((fd, PathBuf::from(pathname)))
> +}
> +
>  /// Takes a Path and CreateOptions, creates a tmpfile from it and returns
>  /// a RawFd and PathBuf for it
>  pub fn make_tmp_file<P: AsRef<Path>>(
> @@ -127,7 +150,7 @@ pub fn make_tmp_file<P: AsRef<Path>>(
>      // use mkstemp here, because it works with different processes, threads, even tokio tasks
>      let mut template = path.to_owned();
>      template.set_extension("tmp_XXXXXX");
> -    let (mut file, tmp_path) = match unistd::mkstemp(&template) {
> +    let (mut file, tmp_path) = match mkostemp(&template, OFlag::O_CLOEXEC) {
>          Ok((fd, path)) => (unsafe { File::from_raw_fd(fd) }, path),
>          Err(err) => bail!("mkstemp {:?} failed: {}", template, err),
>      };
> -- 
> 2.39.5
> 
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
> 
> 
> 


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] applied: [PATCH proxmox v2 1/2] sys: fs: set CLOEXEC when creating temp files

[Thomas Lamprecht]

Am 29.11.24 um 15:28 schrieb Dominik Csapak:
> In general we want all open files to have set CLOEXEC since our
> reloading mechanism can basically fork at any moment and we don't want
> newer daemons to carry around old file descriptors, especially lock
> files.
> 
> Since `make_tmp_file` is called by many things (e.g. open_file_locked,
> logrotate, rrd), set O_CLOEXEC with mkostemp.
> 
> This fixes issues with leftover file descriptors e.g. tape backups not
> working because of lingering locks after a reload, or having deleted
> rrd files open.
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> changes from v1:
> * introduce mkostemp helper which is similar to nix's mkstemp helper
>   (the code is a copy of mkstemp aside from the call to libcmkostemp +
>   the oflag handling)
> 
>   I did it this way, since we may be able to upstream this, have
>   to look more closer at this though.
> 
>  proxmox-sys/src/fs/file.rs | 25 ++++++++++++++++++++++++-
>  1 file changed, 24 insertions(+), 1 deletion(-)
> 
>

applied both patches with Fabian's R-b, thanks!

I amended the doc-comment and commit message of the second patch a bit
though.


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel