Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose who is using a board when it's blocked #79

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+40 −13
Open

Expose who is using a board when it's blocked #79

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
de2f592
make cdba user variable global
calebccff Oct 17, 2024
7a25dee
device: track who is using board
calebccff Oct 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 8 additions & 10 deletions cdba-server.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@
#include "list.h"
#include "watch.h"

static const char *username;

struct device *selected_device;

static void fastboot_opened(struct fastboot *fb, void *data)
Expand Down Expand Up @@ -55,7 +53,7 @@ static struct fastboot_ops fastboot_ops = {

static void msg_select_board(const void *param)
{
selected_device = device_open(param, username);
selected_device = device_open(param, cdba_user);
if (!selected_device) {
fprintf(stderr, "failed to open %s\n", (const char *)param);
watch_quit();
Expand Down Expand Up @@ -177,10 +175,10 @@ static int handle_stdin(int fd, void *buf)
device_send_break(selected_device);
break;
case MSG_LIST_DEVICES:
device_list_devices(username);
device_list_devices(cdba_user);
break;
case MSG_BOARD_INFO:
device_info(username, msg->data, msg->len);
device_info(cdba_user, msg->data, msg->len);
break;
case MSG_FASTBOOT_CONTINUE:
msg_fastboot_continue();
Expand Down Expand Up @@ -215,11 +213,11 @@ int main(int argc, char **argv)

fprintf(stderr, "Starting cdba server\n");

username = getenv("CDBA_USER");
if (!username)
username = getenv("USER");
if (!username)
username = "nobody";
cdba_user = getenv("CDBA_USER");
if (!cdba_user)
cdba_user = getenv("USER");
if (!cdba_user)
cdba_user = "nobody";

openlog("cdba-server", LOG_PID, LOG_DAEMON);
atexit(atexit_handler);
Expand Down
2 changes: 2 additions & 0 deletions cdba-server.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

#include "cdba.h"

extern const char *cdba_user;

void cdba_send_buf(int type, size_t len, const void *buf);
#define cdba_send(type) cdba_send_buf(type, 0, NULL)

Expand Down
33 changes: 30 additions & 3 deletions device.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
#include "status-cmd.h"
#include "watch.h"

const char *cdba_user;

#define ARRAY_SIZE(x) ((sizeof(x)/sizeof((x)[0])))

#define device_has_control(_dev, _op) \
Expand All @@ -49,6 +51,7 @@
static void device_lock(struct device *device)
{
char lock[PATH_MAX];
char user[128] = { 0 };
int fd;
int n;

Expand All @@ -60,25 +63,49 @@
if (fd >= 0)
close(fd);

fd = open(lock, O_RDONLY | O_CLOEXEC);
fd = open(lock, O_RDWR | O_CLOEXEC);
if (fd < 0)
err(1, "failed to open lockfile %s", lock);

/* Read current user out of the lockfile if there is one */
n = read(fd, user, sizeof(user)-1);
Copy link
Collaborator

@lumag lumag Oct 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should happen under the read-lock (so that reading doesn't collide with unfinished write call). But then we don't want to give away exclusive lock on the file. Maybe it's better to have two files: one for the lock and another one for the username.

if (n < 0)
err(1, "failed to read lockfile %s", lock);
/* Strip newline */
if (n)
user[n-1] = '\0';

while (1) {
char c;

n = flock(fd, LOCK_EX | LOCK_NB);
if (!n)
return;
break;

warnx("board is in use, waiting...");
warnx("board is in use by %s, waiting...", user);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

User is not a constant, it needs to be re-read constantly when printing the message. Consider userB "stealing" the board while you are waiting for it to be freed by userA. The message will still print userA, while you should be pinging userB.


sleep(3);

/* check that connection isn't gone */
if (read(STDIN_FILENO, &c, 1) == 0)
errx(1, "connection is gone");
}

/* Write our username to the lockfile */
n = snprintf(user, sizeof(user), "%s\n", cdba_user);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think skipping \n might be a better choice, it saves the code on both read and write sides.

if (n >= (int)sizeof(user))
errx(1, "failed to build lockfile username");

if (ftruncate(fd, 0) < 0)
err(1, "failed to truncate lockfile %s", lock);

lseek(fd, 0, SEEK_SET);
if (write(fd, user, n) < 0)

Check warning

Code scanning / CodeQL

Exposure of system data to an unauthorized control sphere Medium

This operation exposes system data from
*call to getenv
Loading
.
This operation exposes system data from
*call to getenv
Loading
.
err(1, "failed to write lockfile %s", lock);

warnx("board locked by %s", cdba_user);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

debug message?


fsync(fd);
}

static bool device_check_access(struct device *device,
Expand Down
Loading