Skip to content

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

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

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

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
de2f592
make cdba user variable global
kcxt Oct 17, 2024
7a25dee
device: track who is using board
kcxt 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
31 changes: 28 additions & 3 deletions device.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
static void device_lock(struct device *device)
{
char lock[PATH_MAX];
char user[128] = { 0 };
int fd;
int n;

Expand All @@ -62,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

@lumag lumag Oct 17, 2024

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

@lumag lumag Oct 17, 2024

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

@lumag lumag Oct 17, 2024

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