DAOS-18262 common: make sure DAOS file permissions are always 0XXY by janekmi · Pull Request #17680 · daos-stack/daos

janekmi · 2026-03-10T20:32:57Z

At least:

0660 for files
0770 for directories.

So daos_server group can access DAOS files.

Guide MD-on-PMEM

daos/                           src/control/server/storage/mount/provider.go:23
├── 8ac60360-eae4-4071-86ac-a83a42dc22a3        src/mgmt/srv_target.c:699
│   ├── rdb-pool                No change
│   └── vos-X                   src/vos/vos_pool.c:1001
├── control_raft                src/control/system/raft/raft_recovery.go:93
│   ├── daos_system.db          src/control/system/raft/raft.go:269
│   └── snapshots               No change
├── daos_sys                    src/vos/sys_db.c:131
│   └── sys_db                  No change
├── NEWBORNS                    src/mgmt/srv_target.c:452
├── superblock                  src/control/common/file_utils.go:141,src/control/server/instance_superblock.go:208
└── ZOMBIES                     src/mgmt/srv_target.c:460

Guide MD-on-SSD

tree daos/
daos/                           src/control/server/storage/mount/provider.go:23
├── b4defdf5-a92f-45fe-9be0-b96122efa42d        src/mgmt/srv_target.c:699
│   ├── rdb-pool                No change
│   └── vos-X                   src/mgmt/mgmt_common.c:211
├── NEWBORNS                    src/mgmt/mgmt_common.c:134
└── ZOMBIES                     src/mgmt/mgmt_common.c:134

tree config/daos_control/
config/daos_control/            src/control/server/storage/metadata/provider.go:207
├── control_raft                src/control/system/raft/raft_recovery.go:93
│   ├── daos_system.db          src/control/system/raft/raft.go:269
│   └── snapshots               No change
│       └── 2-39-1773083440395
│           ├── meta.json
│           └── state.bin
└── engine0                     src/control/server/storage/metadata/provider.go:233
    ├── daos_nvme.conf          src/control/server/storage/bdev/backend_class.go:106
    ├── daos_sys                src/vos/sys_db.c:131
    │   └── sys_db              No change
    └── superblock              src/control/common/file_utils.go:141,src/control/server/instance_superblock.go:208

Steps for the author:

Commit message follows the guidelines.
Appropriate Features or Test-tag pragmas were used.
Appropriate Functional Test Stages were run.
At least two positive code reviews including at least one code owner from each category referenced in the PR.
Testing is complete. If necessary, forced-landing label added and a reason added in a comment.

After all prior steps are complete:

Gatekeeper requested (daos-gatekeeper added as a reviewer).

At least: - 0660 for files - 0770 for directories. So daos_server group can access DAOS files. Signed-off-by: Jan Michalski <jan-marian.michalski@hpe.com>

github-actions · 2026-03-10T20:42:52Z

Ticket title is 'dlck command seems to require sudo privileges. Also hit the DER_NOMEM (-1009) issue.'
Status is 'Awaiting Verification'
Labels: 'test_2.8'
https://daosio.atlassian.net/browse/DAOS-18262

src/rdb/rdb.c

liw · 2026-03-11T00:11:20Z

src/rdb/rdb.c

 	    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);


Would it be safer and more appropriate to do this in dss_vos_pool_create (actually why not in server_init or daos_engine's main) instead? Is rdb special in some way?

The thing is you can put them in many different places and end up with the intended result. RDB turned out to be special because of the position I put other umask() calls so one rdb_create() call had been left unattended. But no more. I move this call a little higher up the stack so now it covers all necessary contexts. No special case for RDB. I hope you like this way. 🙂

Thank you for responding. I'd think either one approaches the file permission problem from outside of daos_engine or even daos_server (for instance, is there any systemd setting?), or one does it in main once. It's especially concerning if one changes umask to a more permissive value (e.g., 0). That said, I've not investigated into umasks deeply myself.

Since umask is per process, not per thread, changing and restoring it in target threads brings up potential questions on races among the threads---one might not want to get into such a situation.

grom72 · 2026-03-11T13:03:25Z

src/mgmt/srv_target.c

 			goto out;

-		rc = mkdir(tca->tca_newborn, 0700);
+		rc = mkdir(tca->tca_newborn, 0770);


Suggested change

rc = mkdir(tca->tca_newborn, 0770);

rc = mkdir(tca->tca_newborn, S_IRWXU | S_IRWXG);

src/control/server/storage/bdev/backend_class.go

grom72 · 2026-03-11T13:06:38Z

src/control/server/storage/bdev/backend_class.go

 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


Suggested change

defaultAioFileMode = 0660 // AIO file permissions set to owner +rw

defaultAioFileMode = 0660 // AIO file permissions set to owner and group +rw

src/control/system/raft/raft.go

grom72 · 2026-03-11T13:12:30Z

src/mgmt/mgmt_common.c

 		goto out;
 	}
-	rc = mkdir(pool_newborns_path, 0700);
+	rc = mkdir(pool_newborns_path, 0770);


Suggested change

rc = mkdir(pool_newborns_path, 0770);

rc = mkdir(pool_newborns_path, S_IRWXU | S_IRWXG);

grom72 · 2026-03-11T13:13:08Z

src/control/system/raft/raft_recovery.go


 func createRaftDir(dbPath string) error {
-	if err := os.Mkdir(dbPath, 0700); err != nil && !os.IsExist(err) {
+	var perm os.FileMode = 0770


Please be consistent: perms := os.FileMode(0775) was used above

Suggested change

var perm os.FileMode = 0770

perms := os.FileMode(0770)

src/control/server/storage/mount/provider.go

Remove from tgt_create_preallocate() and rdb_create(). Add one in ds_mgmt_create_pool(). Signed-off-by: Jan Michalski <jan-marian.michalski@hpe.com>

daosbuild3 · 2026-03-12T09:48:00Z

Test stage Functional Hardware Medium MD on SSD completed with status UNSTABLE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net/job/daos-stack/job/daos//view/change-requests/job/PR-17680/2/testReport/

tanabarr

LGTM

tanabarr · 2026-03-12T11:03:05Z

src/control/server/storage/metadata/provider.go

 	}

-	if err := p.sys.Mkdir(req.DataPath, 0755); err != nil {
+	if err := p.sys.Mkdir(req.DataPath, perms); err != nil {


Given we repeat this sequence it would be nice to have a helper that performs Mkdir+Chmod to avoid the specified problem where the permissions are reduced by the umask.

Agreed with Tom. Such a function would belong in src/control/common/file_utils.go.

grom72 · 2026-03-12T10:23:36Z

src/vos/sys_db.c

 static int
 db_open_create(struct sys_db *db, bool try_create)
 {
+	const mode_t       db_dir_mode = 0770;


Suggested change

const mode_t db_dir_mode = 0770;

const mode_t db_dir_mode = S_IRWXU | S_IRWXG;

kjacque · 2026-03-12T22:32:27Z

src/control/server/storage/metadata/provider.go

 }

 func (p *Provider) setupDataDir(req storage.MetadataFormatRequest) error {
+	perms := os.FileMode(0775)


Would it make sense to have a constant somewhere indicating these new default perms? Same question applies at the engine level.

If we define the permission values in a C header file for the engines, we can import it in our Go package lib/daos.

kjacque · 2026-03-12T22:33:27Z

src/control/server/storage/metadata/provider.go

 	}

-	if err := p.sys.Mkdir(req.DataPath, 0755); err != nil {
+	if err := p.sys.Mkdir(req.DataPath, perms); err != nil {


Agreed with Tom. Such a function would belong in src/control/common/file_utils.go.

kjacque · 2026-03-12T22:36:15Z

src/control/system/raft/raft.go


+	// 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)


Given how many places this needs to be added, I'd suggest adding SetDefaultFilePerm and SetDefaultDirPerm functions to keep the specifics of these defaults all in one place.

DAOS-18262 common: make sure DAOS file permissions are always 0XXY

8eece70

At least: - 0660 for files - 0770 for directories. So daos_server group can access DAOS files. Signed-off-by: Jan Michalski <jan-marian.michalski@hpe.com>

janekmi requested review from a team as code owners March 10, 2026 20:32

janekmi requested review from rpadma2 and tanabarr March 10, 2026 21:23

liw reviewed Mar 11, 2026

View reviewed changes

janekmi requested a review from grom72 March 11, 2026 09:42

grom72 requested changes Mar 11, 2026

View reviewed changes

DAOS-18262 common: improve umask() calls positions

b72f0bc

Remove from tgt_create_preallocate() and rdb_create(). Add one in ds_mgmt_create_pool(). Signed-off-by: Jan Michalski <jan-marian.michalski@hpe.com>

tanabarr approved these changes Mar 12, 2026

View reviewed changes

grom72 reviewed Mar 12, 2026

View reviewed changes

kjacque requested changes Mar 12, 2026

View reviewed changes

	rc = mkdir(tca->tca_newborn, 0770);
	rc = mkdir(tca->tca_newborn, S_IRWXU \| S_IRWXG);

	defaultAioFileMode = 0660 // AIO file permissions set to owner +rw
	defaultAioFileMode = 0660 // AIO file permissions set to owner and group +rw

	rc = mkdir(pool_newborns_path, 0770);
	rc = mkdir(pool_newborns_path, S_IRWXU \| S_IRWXG);

	const mode_t db_dir_mode = 0770;
	const mode_t db_dir_mode = S_IRWXU \| S_IRWXG;

Conversation

janekmi commented Mar 10, 2026

Guide MD-on-PMEM

Guide MD-on-SSD

Steps for the author:

After all prior steps are complete:

Uh oh!

github-actions bot commented Mar 10, 2026

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

liw Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

daosbuild3 commented Mar 12, 2026

Uh oh!

tanabarr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

6 participants

liw Mar 12, 2026 •

edited

Loading