Skip to content

Commit 931292f

Browse files
committed
misc/user: use lockless update for user_creds structure
Using a reference count on user_creds structure avoids having to hold the process's lock when reading its content. Instead, the accesser uses a kind of RCU mechanism to only read the credentials at the time the function was called. This makes the code much cleaner, and avoids potentially blocking other process operations. NOTE: Since everything is lockless now, this introduces a potential TOCTOU error if 2 threads belonging to the same process call the set[*]uid/gid() function at the same time. This does not seem like a critical issue, but we may want to prevent reads during update operations (true RCU). Closes #72
1 parent 7ef398c commit 931292f

5 files changed

Lines changed: 316 additions & 182 deletions

File tree

include/kernel/process.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ struct process {
107107
thread_state_t state;
108108
uint8_t exit_status; /** Transmitted to the parent process during wait() */
109109

110-
struct user_creds creds; /** Process credentials. */
110+
struct user_creds *creds; /** Process credentials. */
111111

112112
spinlock_t lock;
113113
};

include/kernel/user.h

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,11 @@
1515

1616
#include <kernel/error.h>
1717
#include <kernel/types.h>
18+
#include <kernel/refcnt.h>
1819

1920
/** User credentials. */
2021
struct user_creds {
22+
refcnt_t ref;
2123
uid_t ruid; /*!< Real UID */
2224
uid_t rgid; /*!< Real GID */
2325
uid_t euid; /*!< Effective UID */
@@ -33,8 +35,11 @@ static inline bool creds_is_root(const struct user_creds *creds)
3335
return creds->ruid == UID_ROOT;
3436
}
3537

36-
/** Copy the credentials from @c src into @c dst. */
37-
void creds_copy(struct user_creds *dst, const struct user_creds *src);
38+
struct user_creds *creds_new(void);
39+
struct user_creds *creds_get(struct user_creds *creds);
40+
void creds_put(struct user_creds *creds);
41+
void creds_install(struct user_creds **dest, struct user_creds *creds);
42+
struct user_creds *creds_clone(struct user_creds *creds);
3843

3944
#endif /* KERNEL_USER_H */
4045

kernel/fs/vfs.c

Lines changed: 28 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@ error_t vfs_unmount(const char *path)
138138
static inline struct vnode *
139139
vfs_find_child_at(struct vnode **parent, const path_segment_t *segment)
140140
{
141+
struct user_creds *creds = NULL;
141142
vnode_t *node = *parent;
142143
vnode_t *child;
143144
vfs_t *fs;
@@ -161,21 +162,22 @@ vfs_find_child_at(struct vnode **parent, const path_segment_t *segment)
161162
*parent = node;
162163
}
163164

165+
creds = creds_get(current->process->creds);
166+
164167
child = PTR_ERR(E_NOT_DIRECTORY);
165168
if (node->type != VNODE_DIRECTORY)
166169
goto out;
167170

168171
/* Make sure that the user can search for files inside this directory. */
169-
locked_scope (&current->process->lock) {
170-
child = PTR_ERR(E_ACCESS);
171-
if (!vfs_vnode_check_creds(node, &current->process->creds, O_SEARCH))
172-
goto out;
173-
}
172+
child = PTR_ERR(E_ACCESS);
173+
if (!vfs_vnode_check_creds(node, creds, O_SEARCH))
174+
goto out;
174175

175176
child = node->operations->lookup(node, segment);
176177

177178
out:
178179
spinlock_release(&node->lock);
180+
creds_put(creds);
179181
return child;
180182
}
181183

@@ -243,6 +245,7 @@ static vnode_t *
243245
vfs_create_at(struct vnode *parent, const char *name, vnode_type type)
244246
{
245247
struct vnode *vnode;
248+
struct user_creds *creds;
246249

247250
if (parent->operations->create == NULL)
248251
return PTR_ERR(E_NOT_SUPPORTED);
@@ -251,13 +254,14 @@ vfs_create_at(struct vnode *parent, const char *name, vnode_type type)
251254
if (IS_ERR(vnode))
252255
return vnode;
253256

254-
vnode->stat.st_nlink = 1;
257+
creds = creds_get(current->process->creds);
255258

256259
/* TODO: set file access mode (see opengroup's description of O_CREAT). */
257-
locked_scope (&current->process->lock) {
258-
vnode->stat.st_gid = current->process->creds.egid;
259-
vnode->stat.st_uid = current->process->creds.euid;
260-
}
260+
vnode->stat.st_nlink = 1;
261+
vnode->stat.st_gid = creds->egid;
262+
vnode->stat.st_uid = creds->euid;
263+
264+
creds_put(creds);
261265

262266
return vnode;
263267
}
@@ -366,39 +370,45 @@ static inline error_t file_compute_flags(int oflags, int *flags)
366370
static struct file *vfs_open_at(struct vnode *vnode, int oflags)
367371
{
368372
struct file *file;
373+
struct user_creds *creds;
369374
error_t err;
370375
int flags;
371376

372377
err = file_compute_flags(oflags, &flags);
373378
if (err)
374379
return PTR_ERR(err);
375380

381+
creds = creds_get(current->process->creds);
382+
376383
locked_scope (&vnode->lock) {
377384

385+
file = PTR_ERR(E_NOT_DIRECTORY);
378386
if (vnode->type != VNODE_DIRECTORY) {
379387
if (oflags & O_DIRECTORY)
380-
return PTR_ERR(E_NOT_DIRECTORY);
388+
goto out;
381389
if (oflags & O_SEARCH && O_SEARCH != O_EXEC)
382-
return PTR_ERR(E_NOT_DIRECTORY);
390+
goto out;
383391
}
384392

393+
file = PTR_ERR(E_NOT_SUPPORTED);
385394
if (!vnode->operations->open)
386-
return PTR_ERR(E_NOT_SUPPORTED);
395+
goto out;
387396

388-
locked_scope (&current->process->lock) {
389-
if (!vfs_vnode_check_creds(vnode, &current->process->creds, flags))
390-
return PTR_ERR(E_PERM);
391-
}
397+
file = PTR_ERR(E_PERM);
398+
if (!vfs_vnode_check_creds(vnode, creds, flags))
399+
goto out;
392400

393401
file = vnode->operations->open(vnode);
394402
if (IS_ERR(file))
395-
return file;
403+
goto out;
396404
}
397405

398406
file->flags = flags;
399407
if (flags & FD_APPEND)
400408
file_seek(file, 0, SEEK_END);
401409

410+
out:
411+
creds_put(creds);
402412
return file;
403413
}
404414

0 commit comments

Comments
 (0)