From 8eece7051b51fb72485186cd6bb84c668cdff628 Mon Sep 17 00:00:00 2001 From: Jan Michalski Date: Tue, 10 Mar 2026 20:25:48 +0000 Subject: [PATCH 1/7] DAOS-18262 common: make sure DAOS file permissions are always 0XXY At least: - 0660 for files - 0770 for directories. So daos_server group can access DAOS files. Signed-off-by: Jan Michalski --- src/control/common/file_utils.go | 9 ++++++++- src/control/server/instance_superblock.go | 4 ++-- .../server/storage/bdev/backend_class.go | 11 ++++++++++- .../server/storage/metadata/provider.go | 19 +++++++++++++++++-- src/control/server/storage/mount/provider.go | 6 +++++- src/control/system/raft/raft.go | 9 ++++++++- src/control/system/raft/raft_recovery.go | 7 ++++++- src/mgmt/mgmt_common.c | 6 +++++- src/mgmt/srv_target.c | 9 ++++++--- src/rdb/rdb.c | 4 ++++ src/utils/ddb/ddb_mgmt.c | 4 ++-- src/vos/sys_db.c | 16 ++++++++++++++-- 12 files changed, 87 insertions(+), 17 deletions(-) diff --git a/src/control/common/file_utils.go b/src/control/common/file_utils.go index d0c159dc551..0efcb95db07 100644 --- a/src/control/common/file_utils.go +++ b/src/control/common/file_utils.go @@ -1,6 +1,6 @@ // // (C) Copyright 2019-2024 Intel Corporation. -// (C) Copyright 2025 Hewlett Packard Enterprise Development LP +// (C) Copyright 2025-2026 Hewlett Packard Enterprise Development LP // // SPDX-License-Identifier: BSD-2-Clause-Patent // @@ -139,6 +139,13 @@ func writeFile(path string, data []byte, perm os.FileMode) (err error) { } }() + // The requested permissions may have been reduced by the umask. + // Using chmod ensures the requested permissions are applied. + err = os.Chmod(path, perm) + if err != nil { + return + } + n, err := f.Write(data) if err != nil { return diff --git a/src/control/server/instance_superblock.go b/src/control/server/instance_superblock.go index 4c62f7ccb5c..ccfd339a061 100644 --- a/src/control/server/instance_superblock.go +++ b/src/control/server/instance_superblock.go @@ -1,6 +1,6 @@ // // (C) Copyright 2019-2024 Intel Corporation. -// (C) Copyright 2025 Hewlett Packard Enterprise Development LP +// (C) Copyright 2025-2026 Hewlett Packard Enterprise Development LP // // SPDX-License-Identifier: BSD-2-Clause-Patent // @@ -205,6 +205,6 @@ func WriteSuperblock(sbPath string, sb *Superblock) error { return err } - return errors.Wrapf(common.WriteFileAtomic(sbPath, data, 0600), + return errors.Wrapf(common.WriteFileAtomic(sbPath, data, 0660), "Failed to write Superblock to %s", sbPath) } diff --git a/src/control/server/storage/bdev/backend_class.go b/src/control/server/storage/bdev/backend_class.go index f0d101e5d31..39320837e6e 100644 --- a/src/control/server/storage/bdev/backend_class.go +++ b/src/control/server/storage/bdev/backend_class.go @@ -1,5 +1,6 @@ // // (C) Copyright 2021-2024 Intel Corporation. +// (C) Copyright 2026 Hewlett Packard Enterprise Development LP // (C) Copyright 2025 Google LLC // // SPDX-License-Identifier: BSD-2-Clause-Patent @@ -25,7 +26,7 @@ import ( const ( aioBlockSize = humanize.KiByte * 4 // device block size hardcoded to 4096 bytes - defaultAioFileMode = 0600 // AIO file permissions set to owner +rw + defaultAioFileMode = 0660 // AIO file permissions set to owner +rw ) func createEmptyFile(log logging.Logger, path string, size uint64) error { @@ -102,6 +103,14 @@ func writeConfigFile(log logging.Logger, buf *bytes.Buffer, req *storage.BdevWri return errors.Wrap(err, "create") } + // os.Create() above creates a file with 0666 permissions (before umask). + // Typical umask is 022 so the effective permissions of the created file is 0644. + // The os.Chmod ensures the final permissions for both the user and their group are the same. + err = os.Chmod(req.ConfigOutputPath, 0664) + if err != nil { + return errors.Wrap(err, "chmod") + } + defer func() { if err := f.Close(); err != nil { log.Errorf("closing %q: %s", req.ConfigOutputPath, err) diff --git a/src/control/server/storage/metadata/provider.go b/src/control/server/storage/metadata/provider.go index 07a25157817..87c3d70788a 100644 --- a/src/control/server/storage/metadata/provider.go +++ b/src/control/server/storage/metadata/provider.go @@ -25,6 +25,7 @@ const defaultDevFS = "ext4" type ( // SystemProvider provides operating system capabilities. SystemProvider interface { + Chmod(string, os.FileMode) error Chown(string, int, int) error Getfs(device string) (string, error) GetfsType(path string) (*system.FsType, error) @@ -196,24 +197,38 @@ func (p *Provider) isUsableFS(fs *system.FsType, path string) bool { } func (p *Provider) setupDataDir(req storage.MetadataFormatRequest) error { + perms := os.FileMode(0775) + if err := p.sys.RemoveAll(req.DataPath); err != nil { return errors.Wrap(err, "removing old control metadata subdirectory") } - if err := p.sys.Mkdir(req.DataPath, 0755); err != nil { + if err := p.sys.Mkdir(req.DataPath, perms); err != nil { return errors.Wrap(err, "creating control metadata subdirectory") } + // The requested permissions may have been reduced by the umask. + // Using chmod ensures the requested permissions are applied. + if err := p.sys.Chmod(req.DataPath, perms); err != nil { + return errors.Wrap(err, "setting control metadata subdirectory permissions") + } + if err := p.sys.Chown(req.DataPath, req.OwnerUID, req.OwnerGID); err != nil { return errors.Wrapf(err, "setting ownership of control metadata subdirectory to %d/%d", req.OwnerUID, req.OwnerGID) } for _, idx := range req.EngineIdxs { engPath := storage.ControlMetadataEngineDir(req.DataPath, idx) - if err := p.sys.Mkdir(engPath, 0755); err != nil { + if err := p.sys.Mkdir(engPath, perms); err != nil { return errors.Wrapf(err, "creating control metadata engine %d subdirectory", idx) } + // The requested permissions may have been reduced by the umask. + // Using chmod ensures the final requested are applied. + if err := p.sys.Chmod(engPath, perms); err != nil { + return errors.Wrapf(err, "setting control metadata engine %d subdirectory permissions", idx) + } + if err := p.sys.Chown(engPath, req.OwnerUID, req.OwnerGID); err != nil { return errors.Wrapf(err, "setting ownership of control metadata engine %d subdirectory to %d/%d", idx, req.OwnerUID, req.OwnerGID) } diff --git a/src/control/server/storage/mount/provider.go b/src/control/server/storage/mount/provider.go index c14c7485ff2..ebfb6898a2d 100644 --- a/src/control/server/storage/mount/provider.go +++ b/src/control/server/storage/mount/provider.go @@ -1,5 +1,6 @@ // // (C) Copyright 2022-2024 Intel Corporation. +// (C) Copyright 2026 Hewlett Packard Enterprise Development LP // // SPDX-License-Identifier: BSD-2-Clause-Patent // @@ -19,7 +20,7 @@ import ( ) const ( - defaultMountPointPerms = 0755 + defaultMountPointPerms = 0775 defaultUnmountFlags = 0 ) @@ -176,6 +177,9 @@ func (p *Provider) MakeMountPath(path string, tgtUID, tgtGID int) error { if err := p.sys.Mkdir(ps, defaultMountPointPerms); err != nil { return errors.Wrapf(err, "failed to create directory %q", ps) } + if err := p.sys.Chmod(ps, defaultMountPointPerms); err != nil { + return errors.Wrapf(err, "failed to set directory permissions for %q", ps) + } if err := p.sys.Chown(ps, tgtUID, tgtGID); err != nil { return errors.Wrapf(err, "failed to set ownership of %s to %d.%d from %d.%d", ps, tgtUID, tgtGID, p.sys.Geteuid(), p.sys.Getegid()) diff --git a/src/control/system/raft/raft.go b/src/control/system/raft/raft.go index 35a6535af6e..9f6337bdf5c 100644 --- a/src/control/system/raft/raft.go +++ b/src/control/system/raft/raft.go @@ -1,6 +1,6 @@ // // (C) Copyright 2020-2024 Intel Corporation. -// (C) Copyright 2025 Hewlett Packard Enterprise Development LP +// (C) Copyright 2025-2026 Hewlett Packard Enterprise Development LP // // SPDX-License-Identifier: BSD-2-Clause-Patent // @@ -267,6 +267,13 @@ func ConfigureComponents(log logging.Logger, dbCfg *DatabaseConfig) (*RaftCompon return nil, errors.Wrapf(err, "failed to init boltdb at %s", dbCfg.DBFilePath()) } + // Boltdb file permissions on create are set to 0600. + // The os.Chmod ensures the final permissions for both the user and their group are the same. + err = os.Chmod(dbCfg.DBFilePath(), 0660) + if err != nil { + return nil, errors.Wrapf(err, "failed to set permissions for boltdb at %s", dbCfg.DBFilePath()) + } + return &RaftComponents{ Logger: log, Config: raftCfg, diff --git a/src/control/system/raft/raft_recovery.go b/src/control/system/raft/raft_recovery.go index 0647f43c077..9f1fa5ddadd 100644 --- a/src/control/system/raft/raft_recovery.go +++ b/src/control/system/raft/raft_recovery.go @@ -1,5 +1,6 @@ // // (C) Copyright 2022-2024 Intel Corporation. +// (C) Copyright 2026 Hewlett Packard Enterprise Development LP // // SPDX-License-Identifier: BSD-2-Clause-Patent // @@ -86,9 +87,13 @@ func RecoverLocalReplica(log logging.Logger, cfg *DatabaseConfig) error { } func createRaftDir(dbPath string) error { - if err := os.Mkdir(dbPath, 0700); err != nil && !os.IsExist(err) { + var perm os.FileMode = 0770 + if err := os.Mkdir(dbPath, perm); err != nil && !os.IsExist(err) { return errors.Wrap(err, "failed to create raft directory") } + if err := os.Chmod(dbPath, perm); err != nil { + return errors.Wrap(err, "failed to set permissions for raft directory") + } return nil } diff --git a/src/mgmt/mgmt_common.c b/src/mgmt/mgmt_common.c index 70ffcc46776..3e4e298786c 100644 --- a/src/mgmt/mgmt_common.c +++ b/src/mgmt/mgmt_common.c @@ -90,6 +90,7 @@ ds_mgmt_tgt_recreate(uuid_t pool_uuid, daos_size_t scm_size, int tgt_nr, int *tg char *pool_path = NULL; char *rdb_path = NULL; bool dummy_cancel_state = false; + mode_t stored_mask = umask(0); int rc; int fd; struct stat statbuf; @@ -101,6 +102,7 @@ ds_mgmt_tgt_recreate(uuid_t pool_uuid, daos_size_t scm_size, int tgt_nr, int *tg if (rc) { D_ERROR("pool's path generation failed for " DF_UUID ": " DF_RC "\n", DP_UUID(pool_uuid), DP_RC(rc)); + (void)umask(stored_mask); return rc; } @@ -124,7 +126,7 @@ ds_mgmt_tgt_recreate(uuid_t pool_uuid, daos_size_t scm_size, int tgt_nr, int *tg DP_RC(rc)); goto out; } - rc = mkdir(pool_newborns_path, 0700); + rc = mkdir(pool_newborns_path, 0770); if (rc < 0 && errno != EEXIST) { rc = daos_errno2der(errno); D_ERROR("failed to created pool directory: " DF_RC "\n", DP_RC(rc)); @@ -183,6 +185,8 @@ ds_mgmt_tgt_recreate(uuid_t pool_uuid, daos_size_t scm_size, int tgt_nr, int *tg D_FREE(pool_newborns_path); D_FREE(pool_path); + (void)umask(stored_mask); + return rc; } diff --git a/src/mgmt/srv_target.c b/src/mgmt/srv_target.c index 08db055e5ac..d87acb0ad28 100644 --- a/src/mgmt/srv_target.c +++ b/src/mgmt/srv_target.c @@ -449,7 +449,7 @@ ds_mgmt_tgt_setup(void) stored_mode = umask(0); /** create NEWBORNS directory if it does not exist already */ - rc = mkdir(newborns_path, S_IRWXU); + rc = mkdir(newborns_path, S_IRWXU | S_IRWXG); if (rc < 0 && errno != EEXIST) { D_ERROR("failed to create NEWBORNS dir: %d\n", errno); umask(stored_mode); @@ -457,7 +457,7 @@ ds_mgmt_tgt_setup(void) } /** create ZOMBIES directory if it does not exist already */ - rc = mkdir(zombies_path, S_IRWXU); + rc = mkdir(zombies_path, S_IRWXU | S_IRWXG); if (rc < 0 && errno != EEXIST) { D_ERROR("failed to create ZOMBIES dir: %d\n", errno); umask(stored_mode); @@ -649,6 +649,7 @@ static void * tgt_create_preallocate(void *arg) { struct tgt_create_args *tca = arg; + mode_t stored_mask = umask(0); int rc; (void)dss_xstream_set_affinity(tca->tca_dx); @@ -677,7 +678,7 @@ tgt_create_preallocate(void *arg) if (rc) goto out; - rc = mkdir(tca->tca_newborn, 0700); + rc = mkdir(tca->tca_newborn, 0770); if (rc < 0 && errno != EEXIST) { rc = daos_errno2der(errno); D_ERROR("failed to created pool directory: "DF_RC"\n", @@ -712,6 +713,8 @@ tgt_create_preallocate(void *arg) } out: tca->tca_rc = rc; + (void)umask(stored_mask); + return NULL; } diff --git a/src/rdb/rdb.c b/src/rdb/rdb.c index d40fb39d758..0dc72cbca81 100644 --- a/src/rdb/rdb.c +++ b/src/rdb/rdb.c @@ -10,6 +10,8 @@ #define D_LOGFAC DD_FAC(rdb) +#include + #include #include @@ -39,6 +41,7 @@ rdb_create(const char *path, const uuid_t uuid, uint64_t caller_term, struct rdb_create_params *params, struct rdb_cbs *cbs, void *arg, struct rdb_storage **storagep) { + mode_t stored_mask = umask(0); daos_handle_t pool; daos_handle_t mc; d_iov_t value; @@ -61,6 +64,7 @@ rdb_create(const char *path, const uuid_t uuid, uint64_t caller_term, path, (unsigned char *)uuid, params->rcp_size, 0 /* data_sz */, 0 /* meta_sz */, VOS_POF_SMALL | VOS_POF_EXCL | VOS_POF_RDB | VOS_POF_EXTERNAL_CHKPT, params->rcp_vos_df_version, &pool); + (void)umask(stored_mask); if (rc != 0) goto out; diff --git a/src/utils/ddb/ddb_mgmt.c b/src/utils/ddb/ddb_mgmt.c index 3941168eb48..9efc081941e 100644 --- a/src/utils/ddb/ddb_mgmt.c +++ b/src/utils/ddb/ddb_mgmt.c @@ -229,11 +229,11 @@ ddb_dirs_prepare(const char *path) if (unlikely(rc >= sizeof(zombies_path))) return -DER_EXCEEDS_PATH_LEN; - rc = ddb_mkdir(newborns_path, S_IRWXU); + rc = ddb_mkdir(newborns_path, S_IRWXU | S_IRWXG); if (rc) return rc; - rc = ddb_mkdir(zombies_path, S_IRWXU); + rc = ddb_mkdir(zombies_path, S_IRWXU | S_IRWXG); if (rc) return rc; diff --git a/src/vos/sys_db.c b/src/vos/sys_db.c index 0675de420f2..efdc243a902 100644 --- a/src/vos/sys_db.c +++ b/src/vos/sys_db.c @@ -1,6 +1,6 @@ /** * (C) Copyright 2020-2023 Intel Corporation. - * (C) Copyright 2025 Hewlett Packard Enterprise Development LP + * (C) Copyright 2025-2026 Hewlett Packard Enterprise Development LP * * SPDX-License-Identifier: BSD-2-Clause-Patent */ @@ -106,6 +106,7 @@ db_unlink(struct sys_db *db) static int db_open_create(struct sys_db *db, bool try_create) { + const mode_t db_dir_mode = 0770; struct vos_sys_db *vdb = db2vos(db); d_iov_t key; d_iov_t val; @@ -113,7 +114,7 @@ db_open_create(struct sys_db *db, bool try_create) int rc; if (try_create) { - rc = mkdir(vdb->db_path, 0777); + rc = mkdir(vdb->db_path, db_dir_mode); if (rc < 0 && errno != EEXIST) { rc = daos_errno2der(errno); goto failed; @@ -128,6 +129,17 @@ db_open_create(struct sys_db *db, bool try_create) D_CRIT("No access to existing db file %s\n", vdb->db_file); goto failed; } + /** + * The requested permissions may have been reduced by the umask. + * Using chmod ensures the requested permissions are applied. + */ + rc = chmod(vdb->db_path, db_dir_mode); + if (rc == -1) { + rc = daos_errno2der(errno); + D_CRIT("failed to set directory permissions for %s: " DF_RC "\n", vdb->db_path, + DP_RC(rc)); + goto failed; + } D_DEBUG(DB_IO, "Opening %s, try_create=%d\n", vdb->db_file, try_create); if (try_create) { rc = vos_pool_create(vdb->db_file, vdb->db_pool, SYS_DB_SIZE, 0 /* data_sz */, From b72f0bc8900d9f7bbf03842aa9b20d0c517042ad Mon Sep 17 00:00:00 2001 From: Jan Michalski Date: Wed, 11 Mar 2026 19:26:35 +0000 Subject: [PATCH 2/7] DAOS-18262 common: improve umask() calls positions Remove from tgt_create_preallocate() and rdb_create(). Add one in ds_mgmt_create_pool(). Signed-off-by: Jan Michalski --- src/mgmt/srv_pool.c | 4 ++++ src/mgmt/srv_target.c | 4 +--- src/rdb/rdb.c | 4 ---- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/mgmt/srv_pool.c b/src/mgmt/srv_pool.c index 0a491ec8a7e..43a1c49145c 100644 --- a/src/mgmt/srv_pool.c +++ b/src/mgmt/srv_pool.c @@ -12,6 +12,8 @@ #define D_LOGFAC DD_FAC(mgmt) +#include + #include #include @@ -170,6 +172,7 @@ ds_mgmt_create_pool(uuid_t pool_uuid, const char *group, d_rank_list_t *targets, size_t nvme_size, size_t meta_size, daos_prop_t *prop, d_rank_list_t **svcp, int domains_nr, uint32_t *domains) { + mode_t stored_mask = umask(0); d_rank_list_t *pg_ranks = NULL; d_rank_list_t *pg_targets = NULL; d_rank_list_t *dummy = NULL; @@ -267,6 +270,7 @@ ds_mgmt_create_pool(uuid_t pool_uuid, const char *group, d_rank_list_t *targets, d_rank_list_free(pg_targets); d_rank_list_free(pg_ranks); d_rank_list_free(dummy); + (void)umask(stored_mask); D_DEBUG(DB_MGMT, "create pool "DF_UUID": "DF_RC"\n", DP_UUID(pool_uuid), DP_RC(rc)); return rc; diff --git a/src/mgmt/srv_target.c b/src/mgmt/srv_target.c index d87acb0ad28..aa3ffd5c6a6 100644 --- a/src/mgmt/srv_target.c +++ b/src/mgmt/srv_target.c @@ -648,8 +648,7 @@ struct tgt_create_args { static void * tgt_create_preallocate(void *arg) { - struct tgt_create_args *tca = arg; - mode_t stored_mask = umask(0); + struct tgt_create_args *tca = arg; int rc; (void)dss_xstream_set_affinity(tca->tca_dx); @@ -713,7 +712,6 @@ tgt_create_preallocate(void *arg) } out: tca->tca_rc = rc; - (void)umask(stored_mask); return NULL; } diff --git a/src/rdb/rdb.c b/src/rdb/rdb.c index 0dc72cbca81..d40fb39d758 100644 --- a/src/rdb/rdb.c +++ b/src/rdb/rdb.c @@ -10,8 +10,6 @@ #define D_LOGFAC DD_FAC(rdb) -#include - #include #include @@ -41,7 +39,6 @@ rdb_create(const char *path, const uuid_t uuid, uint64_t caller_term, struct rdb_create_params *params, struct rdb_cbs *cbs, void *arg, struct rdb_storage **storagep) { - mode_t stored_mask = umask(0); daos_handle_t pool; daos_handle_t mc; d_iov_t value; @@ -64,7 +61,6 @@ rdb_create(const char *path, const uuid_t uuid, uint64_t caller_term, path, (unsigned char *)uuid, params->rcp_size, 0 /* data_sz */, 0 /* meta_sz */, VOS_POF_SMALL | VOS_POF_EXCL | VOS_POF_RDB | VOS_POF_EXTERNAL_CHKPT, params->rcp_vos_df_version, &pool); - (void)umask(stored_mask); if (rc != 0) goto out; From 2b76e46a99f808b2e36eda1566a682e13b1b4fa7 Mon Sep 17 00:00:00 2001 From: Jan Michalski Date: Mon, 16 Mar 2026 19:56:49 +0000 Subject: [PATCH 3/7] DAOS-18262 common: 0770 -> S_IRWXU | S_IRWXG (C only) Signed-off-by: Jan Michalski --- src/mgmt/mgmt_common.c | 2 +- src/mgmt/srv_target.c | 2 +- src/vos/sys_db.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/mgmt/mgmt_common.c b/src/mgmt/mgmt_common.c index 3e4e298786c..12d2a479b1f 100644 --- a/src/mgmt/mgmt_common.c +++ b/src/mgmt/mgmt_common.c @@ -126,7 +126,7 @@ ds_mgmt_tgt_recreate(uuid_t pool_uuid, daos_size_t scm_size, int tgt_nr, int *tg DP_RC(rc)); goto out; } - rc = mkdir(pool_newborns_path, 0770); + rc = mkdir(pool_newborns_path, S_IRWXU | S_IRWXG); if (rc < 0 && errno != EEXIST) { rc = daos_errno2der(errno); D_ERROR("failed to created pool directory: " DF_RC "\n", DP_RC(rc)); diff --git a/src/mgmt/srv_target.c b/src/mgmt/srv_target.c index aa3ffd5c6a6..3fb89cf9692 100644 --- a/src/mgmt/srv_target.c +++ b/src/mgmt/srv_target.c @@ -677,7 +677,7 @@ tgt_create_preallocate(void *arg) if (rc) goto out; - rc = mkdir(tca->tca_newborn, 0770); + rc = mkdir(tca->tca_newborn, S_IRWXU | S_IRWXG); if (rc < 0 && errno != EEXIST) { rc = daos_errno2der(errno); D_ERROR("failed to created pool directory: "DF_RC"\n", diff --git a/src/vos/sys_db.c b/src/vos/sys_db.c index efdc243a902..b90f4049545 100644 --- a/src/vos/sys_db.c +++ b/src/vos/sys_db.c @@ -106,7 +106,7 @@ db_unlink(struct sys_db *db) static int db_open_create(struct sys_db *db, bool try_create) { - const mode_t db_dir_mode = 0770; + const mode_t db_dir_mode = S_IRWXU | S_IRWXG; struct vos_sys_db *vdb = db2vos(db); d_iov_t key; d_iov_t val; From 1d6435b421638f4536bd81d3cf2570f78d7a7b67 Mon Sep 17 00:00:00 2001 From: Jan Michalski Date: Mon, 16 Mar 2026 19:58:17 +0000 Subject: [PATCH 4/7] DAOS-18262 common: introduce Default*Perm and Mkdir2 Add two constants and apply where possible: - DefaultFilePerm and - DefaultDirPerm Define Mkdir2() as Mkdir() + Chmod(). Signed-off-by: Jan Michalski --- src/control/common/file_utils.go | 21 ++++++++++++++++--- src/control/provider/system/mocks.go | 11 +++++++++- src/control/provider/system/system_linux.go | 6 ++++++ src/control/server/instance_superblock.go | 2 +- .../server/storage/bdev/backend_class.go | 4 ++-- .../server/storage/metadata/provider.go | 19 +++-------------- src/control/server/storage/mount/provider.go | 7 ++----- src/control/system/raft/raft.go | 2 +- src/control/system/raft/raft_recovery.go | 7 ++----- 9 files changed, 45 insertions(+), 34 deletions(-) diff --git a/src/control/common/file_utils.go b/src/control/common/file_utils.go index 0efcb95db07..18267f3f9e2 100644 --- a/src/control/common/file_utils.go +++ b/src/control/common/file_utils.go @@ -20,9 +20,13 @@ import ( yaml "gopkg.in/yaml.v2" ) -// UtilLogDepth signifies stack depth, set calldepth on calls to logger so -// log message context refers to caller not callee. -const UtilLogDepth = 4 +const ( + // UtilLogDepth signifies stack depth, set calldepth on calls to logger so + // log message context refers to caller not callee. + UtilLogDepth = 4 + DefaultFilePerm = 0660 + DefaultDirPerm = 0770 +) // GetFilenames returns names of files in a directory. func GetFilenames(dir string) ([]string, error) { @@ -377,3 +381,14 @@ func HasPrefixPath(base, sub string) (bool, error) { return true, nil } + +// Mkdir2 creates a new directory with the specified name and permission bits (umask ignored). +func Mkdir2(path string, perm os.FileMode) error { + err := os.Mkdir(path, perm) + if err != nil { + return err + } + // The requested permissions may have been reduced by the umask. + // Using Chmod ensures the requested permissions are applied. + return os.Chmod(path, perm) +} diff --git a/src/control/provider/system/mocks.go b/src/control/provider/system/mocks.go index 52384054f8a..76523cfbf52 100644 --- a/src/control/provider/system/mocks.go +++ b/src/control/provider/system/mocks.go @@ -1,6 +1,6 @@ // // (C) Copyright 2022-2024 Intel Corporation. -// (C) Copyright 2025 Hewlett Packard Enterprise Development LP +// (C) Copyright 2025-2026 Hewlett Packard Enterprise Development LP // // SPDX-License-Identifier: BSD-2-Clause-Patent // @@ -12,6 +12,7 @@ import ( "strings" "sync" + "github.com/daos-stack/daos/src/control/common" "github.com/daos-stack/daos/src/control/logging" ) @@ -247,6 +248,14 @@ func (msp *MockSysProvider) Mkdir(path string, flags os.FileMode) error { return msp.cfg.MkdirErr } +// Mkdir2 creates a new directory with the specified name and permission bits (umask ignored). +func (msp *MockSysProvider) Mkdir2(path string, flags os.FileMode) error { + if msp.cfg.RealMkdir { + return common.Mkdir2(path, flags) + } + return msp.cfg.MkdirErr +} + // RemoveAll removes path and any children it contains. func (msp *MockSysProvider) RemoveAll(path string) error { if msp.cfg.RealRemoveAll { diff --git a/src/control/provider/system/system_linux.go b/src/control/provider/system/system_linux.go index 846e2f4ca50..ecabf86b1e4 100644 --- a/src/control/provider/system/system_linux.go +++ b/src/control/provider/system/system_linux.go @@ -17,6 +17,7 @@ import ( "strings" "syscall" + "github.com/daos-stack/daos/src/control/common" "github.com/pkg/errors" "golang.org/x/sys/unix" ) @@ -397,6 +398,11 @@ func (s LinuxProvider) Mkdir(name string, perm os.FileMode) error { return os.Mkdir(name, perm) } +// Mkdir2 creates a new directory with the specified name and permission bits (umask ignored). +func (s LinuxProvider) Mkdir2(path string, perm os.FileMode) error { + return common.Mkdir2(path, perm) +} + // RemoveAll removes path and any children it contains. func (s LinuxProvider) RemoveAll(path string) error { return os.RemoveAll(path) diff --git a/src/control/server/instance_superblock.go b/src/control/server/instance_superblock.go index ccfd339a061..8bd249d8387 100644 --- a/src/control/server/instance_superblock.go +++ b/src/control/server/instance_superblock.go @@ -205,6 +205,6 @@ func WriteSuperblock(sbPath string, sb *Superblock) error { return err } - return errors.Wrapf(common.WriteFileAtomic(sbPath, data, 0660), + return errors.Wrapf(common.WriteFileAtomic(sbPath, data, common.DefaultFilePerm), "Failed to write Superblock to %s", sbPath) } diff --git a/src/control/server/storage/bdev/backend_class.go b/src/control/server/storage/bdev/backend_class.go index 39320837e6e..5977bec6eac 100644 --- a/src/control/server/storage/bdev/backend_class.go +++ b/src/control/server/storage/bdev/backend_class.go @@ -25,8 +25,8 @@ import ( ) const ( - aioBlockSize = humanize.KiByte * 4 // device block size hardcoded to 4096 bytes - defaultAioFileMode = 0660 // AIO file permissions set to owner +rw + aioBlockSize = humanize.KiByte * 4 // device block size hardcoded to 4096 bytes + defaultAioFileMode = common.DefaultFilePerm // AIO file permissions ) func createEmptyFile(log logging.Logger, path string, size uint64) error { diff --git a/src/control/server/storage/metadata/provider.go b/src/control/server/storage/metadata/provider.go index 87c3d70788a..e7af555093f 100644 --- a/src/control/server/storage/metadata/provider.go +++ b/src/control/server/storage/metadata/provider.go @@ -25,12 +25,11 @@ const defaultDevFS = "ext4" type ( // SystemProvider provides operating system capabilities. SystemProvider interface { - Chmod(string, os.FileMode) error Chown(string, int, int) error Getfs(device string) (string, error) GetfsType(path string) (*system.FsType, error) GetDeviceLabel(string) (string, error) - Mkdir(string, os.FileMode) error + Mkdir2(string, os.FileMode) error Mkfs(req system.MkfsReq) error RemoveAll(string) error Stat(string) (os.FileInfo, error) @@ -203,32 +202,20 @@ func (p *Provider) setupDataDir(req storage.MetadataFormatRequest) error { return errors.Wrap(err, "removing old control metadata subdirectory") } - if err := p.sys.Mkdir(req.DataPath, perms); err != nil { + if err := p.sys.Mkdir2(req.DataPath, perms); err != nil { return errors.Wrap(err, "creating control metadata subdirectory") } - // The requested permissions may have been reduced by the umask. - // Using chmod ensures the requested permissions are applied. - if err := p.sys.Chmod(req.DataPath, perms); err != nil { - return errors.Wrap(err, "setting control metadata subdirectory permissions") - } - if err := p.sys.Chown(req.DataPath, req.OwnerUID, req.OwnerGID); err != nil { return errors.Wrapf(err, "setting ownership of control metadata subdirectory to %d/%d", req.OwnerUID, req.OwnerGID) } for _, idx := range req.EngineIdxs { engPath := storage.ControlMetadataEngineDir(req.DataPath, idx) - if err := p.sys.Mkdir(engPath, perms); err != nil { + if err := p.sys.Mkdir2(engPath, perms); err != nil { return errors.Wrapf(err, "creating control metadata engine %d subdirectory", idx) } - // The requested permissions may have been reduced by the umask. - // Using chmod ensures the final requested are applied. - if err := p.sys.Chmod(engPath, perms); err != nil { - return errors.Wrapf(err, "setting control metadata engine %d subdirectory permissions", idx) - } - if err := p.sys.Chown(engPath, req.OwnerUID, req.OwnerGID); err != nil { return errors.Wrapf(err, "setting ownership of control metadata engine %d subdirectory to %d/%d", idx, req.OwnerUID, req.OwnerGID) } diff --git a/src/control/server/storage/mount/provider.go b/src/control/server/storage/mount/provider.go index ebfb6898a2d..c72eca54068 100644 --- a/src/control/server/storage/mount/provider.go +++ b/src/control/server/storage/mount/provider.go @@ -34,7 +34,7 @@ type ( Chown(string, int, int) error Getegid() int Geteuid() int - Mkdir(string, os.FileMode) error + Mkdir2(string, os.FileMode) error RemoveAll(string) error Stat(string) (os.FileInfo, error) } @@ -174,12 +174,9 @@ func (p *Provider) MakeMountPath(path string, tgtUID, tgtGID int) error { switch { case os.IsNotExist(err): // subdir missing, attempt to create and chown - if err := p.sys.Mkdir(ps, defaultMountPointPerms); err != nil { + if err := p.sys.Mkdir2(ps, defaultMountPointPerms); err != nil { return errors.Wrapf(err, "failed to create directory %q", ps) } - if err := p.sys.Chmod(ps, defaultMountPointPerms); err != nil { - return errors.Wrapf(err, "failed to set directory permissions for %q", ps) - } if err := p.sys.Chown(ps, tgtUID, tgtGID); err != nil { return errors.Wrapf(err, "failed to set ownership of %s to %d.%d from %d.%d", ps, tgtUID, tgtGID, p.sys.Geteuid(), p.sys.Getegid()) diff --git a/src/control/system/raft/raft.go b/src/control/system/raft/raft.go index 9f6337bdf5c..17a57322957 100644 --- a/src/control/system/raft/raft.go +++ b/src/control/system/raft/raft.go @@ -269,7 +269,7 @@ func ConfigureComponents(log logging.Logger, dbCfg *DatabaseConfig) (*RaftCompon // Boltdb file permissions on create are set to 0600. // The os.Chmod ensures the final permissions for both the user and their group are the same. - err = os.Chmod(dbCfg.DBFilePath(), 0660) + err = os.Chmod(dbCfg.DBFilePath(), common.DefaultFilePerm) if err != nil { return nil, errors.Wrapf(err, "failed to set permissions for boltdb at %s", dbCfg.DBFilePath()) } diff --git a/src/control/system/raft/raft_recovery.go b/src/control/system/raft/raft_recovery.go index 9f1fa5ddadd..9741150eff1 100644 --- a/src/control/system/raft/raft_recovery.go +++ b/src/control/system/raft/raft_recovery.go @@ -22,6 +22,7 @@ import ( "github.com/pkg/errors" "go.etcd.io/bbolt" + "github.com/daos-stack/daos/src/control/common" "github.com/daos-stack/daos/src/control/lib/ranklist" "github.com/daos-stack/daos/src/control/logging" "github.com/daos-stack/daos/src/control/system" @@ -87,13 +88,9 @@ func RecoverLocalReplica(log logging.Logger, cfg *DatabaseConfig) error { } func createRaftDir(dbPath string) error { - var perm os.FileMode = 0770 - if err := os.Mkdir(dbPath, perm); err != nil && !os.IsExist(err) { + if err := common.Mkdir2(dbPath, common.DefaultDirPerm); err != nil && !os.IsExist(err) { return errors.Wrap(err, "failed to create raft directory") } - if err := os.Chmod(dbPath, perm); err != nil { - return errors.Wrap(err, "failed to set permissions for raft directory") - } return nil } From 11cdd5789288041bfd2e52849fce05b2f51ea68f Mon Sep 17 00:00:00 2001 From: Jan Michalski Date: Tue, 17 Mar 2026 14:52:37 +0000 Subject: [PATCH 5/7] DAOS-18262 common: introduce DEFAULT_*_PERM and use them... ... both for C and Go. Signed-off-by: Jan Michalski --- src/control/common/file_utils.go | 9 +++++++-- src/include/daos_srv/control.h | 4 ++++ src/mgmt/mgmt_common.c | 2 +- src/mgmt/srv_target.c | 6 +++--- src/utils/ddb/ddb_mgmt.c | 4 ++-- src/vos/sys_db.c | 5 ++--- 6 files changed, 19 insertions(+), 11 deletions(-) diff --git a/src/control/common/file_utils.go b/src/control/common/file_utils.go index 18267f3f9e2..e9250567b36 100644 --- a/src/control/common/file_utils.go +++ b/src/control/common/file_utils.go @@ -7,6 +7,11 @@ package common +/* +#include "daos_srv/control.h" +*/ +import "C" + import ( "fmt" "io" @@ -24,8 +29,8 @@ const ( // UtilLogDepth signifies stack depth, set calldepth on calls to logger so // log message context refers to caller not callee. UtilLogDepth = 4 - DefaultFilePerm = 0660 - DefaultDirPerm = 0770 + DefaultFilePerm = C.DEFAULT_FILE_PERM + DefaultDirPerm = C.DEFAULT_DIR_PERM ) // GetFilenames returns names of files in a directory. diff --git a/src/include/daos_srv/control.h b/src/include/daos_srv/control.h index b977fb69f7f..8c043052f08 100644 --- a/src/include/daos_srv/control.h +++ b/src/include/daos_srv/control.h @@ -16,8 +16,12 @@ #include #include #include +#include #include +#define DEFAULT_FILE_PERM (S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP) +#define DEFAULT_DIR_PERM (S_IRWXU | S_IRWXG) + #define NVME_PCI_DEV_TYPE_VMD "vmd" #define NVME_DETAIL_BUFLEN 1024 /** diff --git a/src/mgmt/mgmt_common.c b/src/mgmt/mgmt_common.c index 12d2a479b1f..4a0311c30fc 100644 --- a/src/mgmt/mgmt_common.c +++ b/src/mgmt/mgmt_common.c @@ -126,7 +126,7 @@ ds_mgmt_tgt_recreate(uuid_t pool_uuid, daos_size_t scm_size, int tgt_nr, int *tg DP_RC(rc)); goto out; } - rc = mkdir(pool_newborns_path, S_IRWXU | S_IRWXG); + rc = mkdir(pool_newborns_path, DEFAULT_DIR_PERM); if (rc < 0 && errno != EEXIST) { rc = daos_errno2der(errno); D_ERROR("failed to created pool directory: " DF_RC "\n", DP_RC(rc)); diff --git a/src/mgmt/srv_target.c b/src/mgmt/srv_target.c index 3fb89cf9692..e82d32d5713 100644 --- a/src/mgmt/srv_target.c +++ b/src/mgmt/srv_target.c @@ -449,7 +449,7 @@ ds_mgmt_tgt_setup(void) stored_mode = umask(0); /** create NEWBORNS directory if it does not exist already */ - rc = mkdir(newborns_path, S_IRWXU | S_IRWXG); + rc = mkdir(newborns_path, DEFAULT_DIR_PERM); if (rc < 0 && errno != EEXIST) { D_ERROR("failed to create NEWBORNS dir: %d\n", errno); umask(stored_mode); @@ -457,7 +457,7 @@ ds_mgmt_tgt_setup(void) } /** create ZOMBIES directory if it does not exist already */ - rc = mkdir(zombies_path, S_IRWXU | S_IRWXG); + rc = mkdir(zombies_path, DEFAULT_DIR_PERM); if (rc < 0 && errno != EEXIST) { D_ERROR("failed to create ZOMBIES dir: %d\n", errno); umask(stored_mode); @@ -677,7 +677,7 @@ tgt_create_preallocate(void *arg) if (rc) goto out; - rc = mkdir(tca->tca_newborn, S_IRWXU | S_IRWXG); + rc = mkdir(tca->tca_newborn, DEFAULT_DIR_PERM); if (rc < 0 && errno != EEXIST) { rc = daos_errno2der(errno); D_ERROR("failed to created pool directory: "DF_RC"\n", diff --git a/src/utils/ddb/ddb_mgmt.c b/src/utils/ddb/ddb_mgmt.c index 9efc081941e..668fa2d30e4 100644 --- a/src/utils/ddb/ddb_mgmt.c +++ b/src/utils/ddb/ddb_mgmt.c @@ -229,11 +229,11 @@ ddb_dirs_prepare(const char *path) if (unlikely(rc >= sizeof(zombies_path))) return -DER_EXCEEDS_PATH_LEN; - rc = ddb_mkdir(newborns_path, S_IRWXU | S_IRWXG); + rc = ddb_mkdir(newborns_path, DEFAULT_DIR_PERM); if (rc) return rc; - rc = ddb_mkdir(zombies_path, S_IRWXU | S_IRWXG); + rc = ddb_mkdir(zombies_path, DEFAULT_DIR_PERM); if (rc) return rc; diff --git a/src/vos/sys_db.c b/src/vos/sys_db.c index b90f4049545..38e3359bafa 100644 --- a/src/vos/sys_db.c +++ b/src/vos/sys_db.c @@ -106,7 +106,6 @@ db_unlink(struct sys_db *db) static int db_open_create(struct sys_db *db, bool try_create) { - const mode_t db_dir_mode = S_IRWXU | S_IRWXG; struct vos_sys_db *vdb = db2vos(db); d_iov_t key; d_iov_t val; @@ -114,7 +113,7 @@ db_open_create(struct sys_db *db, bool try_create) int rc; if (try_create) { - rc = mkdir(vdb->db_path, db_dir_mode); + rc = mkdir(vdb->db_path, DEFAULT_DIR_PERM); if (rc < 0 && errno != EEXIST) { rc = daos_errno2der(errno); goto failed; @@ -133,7 +132,7 @@ db_open_create(struct sys_db *db, bool try_create) * The requested permissions may have been reduced by the umask. * Using chmod ensures the requested permissions are applied. */ - rc = chmod(vdb->db_path, db_dir_mode); + rc = chmod(vdb->db_path, DEFAULT_DIR_PERM); if (rc == -1) { rc = daos_errno2der(errno); D_CRIT("failed to set directory permissions for %s: " DF_RC "\n", vdb->db_path, From a0643f466869526df2c067a8fbc76559b9ed1be1 Mon Sep 17 00:00:00 2001 From: Jan Michalski Date: Tue, 17 Mar 2026 15:56:49 +0000 Subject: [PATCH 6/7] DAOS-18262 common: add a nMkdir2 unit test Signed-off-by: Jan Michalski --- src/control/common/file_utils.go | 1 + src/control/common/file_utils_test.go | 23 +++++++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/src/control/common/file_utils.go b/src/control/common/file_utils.go index e9250567b36..be423cf7485 100644 --- a/src/control/common/file_utils.go +++ b/src/control/common/file_utils.go @@ -8,6 +8,7 @@ package common /* +#cgo CFLAGS: -I${SRCDIR}/../../include/ #include "daos_srv/control.h" */ import "C" diff --git a/src/control/common/file_utils_test.go b/src/control/common/file_utils_test.go index 7ed90e12717..a3b58968cb7 100644 --- a/src/control/common/file_utils_test.go +++ b/src/control/common/file_utils_test.go @@ -1,5 +1,6 @@ // // (C) Copyright 2019-2022 Intel Corporation. +// (C) Copyright 2026 Hewlett Packard Enterprise Development LP // // SPDX-License-Identifier: BSD-2-Clause-Patent // @@ -176,5 +177,27 @@ func TestUtils_HasPrefixPath(t *testing.T) { if hp { t.Fatalf("%q is not a prefix of %q", testDir, testPath) } +} + +func TestUtils_Mkdir2(t *testing.T) { + exp_perm := 0777 + + testDir, clean := CreateTestDir(t) + defer clean() + testPath := path.Join(testDir, "foo") + err := Mkdir2(testPath, os.FileMode(exp_perm)) + if err != nil { + t.Fatalf("Unexpected error: %q", err) + } + + info, err := os.Stat(testPath) + if err != nil { + t.Fatalf("Unexpected error: %q", err) + } + + perm := int(info.Mode().Perm()) + if perm != exp_perm { + t.Fatalf("Created directory has unexpected permissions: %04o instead of %04o", perm, exp_perm) + } } From 7d42b83aae89beaa33da996462d50ac102b43ded Mon Sep 17 00:00:00 2001 From: Jan Michalski Date: Tue, 17 Mar 2026 16:55:20 +0000 Subject: [PATCH 7/7] DAOS-18262 common: call umask(002) in server_init() - remove other umask() calls - remove chmod() calls Signed-off-by: Jan Michalski --- src/engine/init.c | 7 +++++++ src/mgmt/mgmt_common.c | 5 ----- src/mgmt/srv_pool.c | 4 ---- src/vos/sys_db.c | 15 ++------------- 4 files changed, 9 insertions(+), 22 deletions(-) diff --git a/src/engine/init.c b/src/engine/init.c index 0072b87be33..b29fac7c55b 100644 --- a/src/engine/init.c +++ b/src/engine/init.c @@ -670,6 +670,13 @@ server_init(int argc, char *argv[]) int rc; struct engine_metrics *metrics; + /** + * The typical umask is 022. The group portion is cleared, which allows the group + * permissions to be set freely. This setting is intended to remain in effect for the entire + * lifetime of the process. + */ + (void)umask(002); + /* * Begin the HLC recovery as early as possible. Do not read the HLC * before the hlc_recovery_end call below. diff --git a/src/mgmt/mgmt_common.c b/src/mgmt/mgmt_common.c index 4a0311c30fc..df7764f1b4f 100644 --- a/src/mgmt/mgmt_common.c +++ b/src/mgmt/mgmt_common.c @@ -12,7 +12,6 @@ #include #include #include -#include #include #include @@ -90,7 +89,6 @@ ds_mgmt_tgt_recreate(uuid_t pool_uuid, daos_size_t scm_size, int tgt_nr, int *tg char *pool_path = NULL; char *rdb_path = NULL; bool dummy_cancel_state = false; - mode_t stored_mask = umask(0); int rc; int fd; struct stat statbuf; @@ -102,7 +100,6 @@ ds_mgmt_tgt_recreate(uuid_t pool_uuid, daos_size_t scm_size, int tgt_nr, int *tg if (rc) { D_ERROR("pool's path generation failed for " DF_UUID ": " DF_RC "\n", DP_UUID(pool_uuid), DP_RC(rc)); - (void)umask(stored_mask); return rc; } @@ -185,8 +182,6 @@ ds_mgmt_tgt_recreate(uuid_t pool_uuid, daos_size_t scm_size, int tgt_nr, int *tg D_FREE(pool_newborns_path); D_FREE(pool_path); - (void)umask(stored_mask); - return rc; } diff --git a/src/mgmt/srv_pool.c b/src/mgmt/srv_pool.c index 43a1c49145c..0a491ec8a7e 100644 --- a/src/mgmt/srv_pool.c +++ b/src/mgmt/srv_pool.c @@ -12,8 +12,6 @@ #define D_LOGFAC DD_FAC(mgmt) -#include - #include #include @@ -172,7 +170,6 @@ ds_mgmt_create_pool(uuid_t pool_uuid, const char *group, d_rank_list_t *targets, size_t nvme_size, size_t meta_size, daos_prop_t *prop, d_rank_list_t **svcp, int domains_nr, uint32_t *domains) { - mode_t stored_mask = umask(0); d_rank_list_t *pg_ranks = NULL; d_rank_list_t *pg_targets = NULL; d_rank_list_t *dummy = NULL; @@ -270,7 +267,6 @@ ds_mgmt_create_pool(uuid_t pool_uuid, const char *group, d_rank_list_t *targets, d_rank_list_free(pg_targets); d_rank_list_free(pg_ranks); d_rank_list_free(dummy); - (void)umask(stored_mask); D_DEBUG(DB_MGMT, "create pool "DF_UUID": "DF_RC"\n", DP_UUID(pool_uuid), DP_RC(rc)); return rc; diff --git a/src/vos/sys_db.c b/src/vos/sys_db.c index 38e3359bafa..0675de420f2 100644 --- a/src/vos/sys_db.c +++ b/src/vos/sys_db.c @@ -1,6 +1,6 @@ /** * (C) Copyright 2020-2023 Intel Corporation. - * (C) Copyright 2025-2026 Hewlett Packard Enterprise Development LP + * (C) Copyright 2025 Hewlett Packard Enterprise Development LP * * SPDX-License-Identifier: BSD-2-Clause-Patent */ @@ -113,7 +113,7 @@ db_open_create(struct sys_db *db, bool try_create) int rc; if (try_create) { - rc = mkdir(vdb->db_path, DEFAULT_DIR_PERM); + rc = mkdir(vdb->db_path, 0777); if (rc < 0 && errno != EEXIST) { rc = daos_errno2der(errno); goto failed; @@ -128,17 +128,6 @@ db_open_create(struct sys_db *db, bool try_create) D_CRIT("No access to existing db file %s\n", vdb->db_file); goto failed; } - /** - * The requested permissions may have been reduced by the umask. - * Using chmod ensures the requested permissions are applied. - */ - rc = chmod(vdb->db_path, DEFAULT_DIR_PERM); - if (rc == -1) { - rc = daos_errno2der(errno); - D_CRIT("failed to set directory permissions for %s: " DF_RC "\n", vdb->db_path, - DP_RC(rc)); - goto failed; - } D_DEBUG(DB_IO, "Opening %s, try_create=%d\n", vdb->db_file, try_create); if (try_create) { rc = vos_pool_create(vdb->db_file, vdb->db_pool, SYS_DB_SIZE, 0 /* data_sz */,