Skip to content

userdel: fix user busy detection for threads #1623

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
haxtibal wants to merge 2 commits into
base: master
Choose a base branch
from
+8 −9
Open

userdel: fix user busy detection for threads #1623

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
821db7c
userdel: fix user busy detection for threads
haxtibal May 6, 2026
a4681d8
lib: simplify check_status and different_namespace interface
haxtibal May 6, 2026
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
17 changes: 8 additions & 9 deletions lib/user_busy.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@


#ifdef __linux__
static int check_status (const char *name, const char *sname, uid_t uid);
static int check_status (const char *name, uid_t uid, pid_t pid, pid_t tid);
static int user_busy_processes (const char *name, uid_t uid);
#else /* !__linux__ */
static int user_busy_utmp (const char *name);
Expand Down Expand Up @@ -94,13 +94,13 @@ user_busy_utmp(const char *name)
#ifdef __linux__
#ifdef ENABLE_SUBIDS
#define in_parentuid_range(uid) ((uid) >= parentuid && (uid) < parentuid + range)
static int different_namespace (const char *sname)
static int different_namespace (pid_t pid, pid_t tid)
{
Comment thread
haxtibal marked this conversation as resolved.
/* 41: /proc/xxxxxxxxxx/task/xxxxxxxxxx/ns/user + \0 */
char path[41];
char buf[512], buf2[512];

stprintf_a(path, "/proc/%s/ns/user", sname);
stprintf_a(path, "/proc/%d/task/%d/ns/user", pid, tid);
Copy link
Copy Markdown
Collaborator

@alejandro-colomar alejandro-colomar May 6, 2026

Choose a reason for hiding this comment

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

As far as I know, pid_t is not required by POSIX to be int. If we want to use %d, we should at least have -Wformat=2 -Werror=format to make sure this doesn't produce UB. Alternatively, you may want to use intmax_t for printing, although if we're pretty confident that pid_t is an int, %d should be good if combined with the compiler error.

Copy link
Copy Markdown
Author

@haxtibal haxtibal May 7, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

As far as I know, pid_t is not required by POSIX to be int

This is true for POSIX, but we're now inside #ifdef __linux__, and for Linux we have

typedef int		__kernel_pid_t;

for all platforms (see uapi/asm-generic/posix_types.h). Further, the existing code also uses %d for pid_t in some places.

However the existing code does it inconsistently. If you tell what's your preference I could add a commit that aligns pid_t formatting for the whole user_busy.c file.

Copy link
Copy Markdown
Collaborator

@alejandro-colomar alejandro-colomar May 7, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

I like using %d. However, I'd like to have a separate commit adding the compiler error so that we make sure we don't have UB.

Consider the possibility that we paste that code somewhere else that is not wrapped in #ifdef __linux__. It would be an easy and dangerous bug. If the compiler prevents us from doing that thing, I'll be happier.


if (readlinknul_a(path, buf) == -1)
return 0;
Expand All @@ -116,14 +116,14 @@ static int different_namespace (const char *sname)
#endif /* ENABLE_SUBIDS */


static int check_status (const char *name, const char *sname, uid_t uid)
static int check_status (const char *name, uid_t uid, pid_t pid, pid_t tid)
{
/* 40: /proc/xxxxxxxxxx/task/xxxxxxxxxx/status + \0 */
char status[40];
char line[1024];
FILE *sfile;

stprintf_a(status, "/proc/%s/status", sname);
stprintf_a(status, "/proc/%d/task/%d/status", pid, tid);

sfile = fopen (status, "r");
if (NULL == sfile) {
Expand All @@ -144,7 +144,7 @@ static int check_status (const char *name, const char *sname, uid_t uid)
return 1;
}
#ifdef ENABLE_SUBIDS
if ( different_namespace (sname)
if ( different_namespace (pid, tid)
&& ( have_sub_uids(name, ruid, 1)
|| have_sub_uids(name, euid, 1)
|| have_sub_uids(name, suid, 1))
Expand Down Expand Up @@ -225,7 +225,7 @@ static int user_busy_processes (const char *name, uid_t uid)
continue;
}

if (check_status (name, tmp_d_name, uid) != 0) {
if (check_status (name, uid, pid, pid) != 0) {
(void) closedir (proc);
#ifdef ENABLE_SUBIDS
sub_uid_close(true);
Expand All @@ -247,7 +247,7 @@ static int user_busy_processes (const char *name, uid_t uid)
if (tid == pid) {
continue;
}
if (check_status (name, task_path+6, uid) != 0) {
if (check_status (name, uid, pid, tid) != 0) {
(void) closedir (proc);
(void) closedir (task_dir);
#ifdef ENABLE_SUBIDS
Expand All @@ -272,4 +272,3 @@ static int user_busy_processes (const char *name, uid_t uid)
return 0;
}
#endif /* __linux__ */

Loading