Re: [PATCH v2] tools/power/x86/intel-speed-select: Harden daemon pidfile open

Re: [PATCH v2] tools/power/x86/intel-speed-select: Harden daemon pidfile open

[Ilpo Järvinen]

On Thu, 23 Apr 2026, unknownbbqrx wrote:

> 
> From: ali <dev@unknownbbqr.xyz>
> 
> Avoid symlink-based pidfile clobbering by opening the pidfile with
> O_NOFOLLOW and validating it with fstat() before locking/writing.
> 
> The daemon currently uses a fixed pidfile path under /tmp. A local
> unprivileged user can pre-create a symlink at that path and cause a
> root-run daemon instance to write into an attacker-chosen file.
> 

Shouldn't this change have a Fixes tag?

> Signed-off-by: ali <dev@unknownbbqr.xyz>

Please use your full name for signing off changes (see 
Documentation/process/submitting-patches.rst).

> ---
>  tools/power/x86/intel-speed-select/isst-daemon.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/power/x86/intel-speed-select/isst-daemon.c b/tools/power/x86/intel-speed-select/isst-daemon.c
> index 66df21b2b..4346b049d 100644
> --- a/tools/power/x86/intel-speed-select/isst-daemon.c
> +++ b/tools/power/x86/intel-speed-select/isst-daemon.c
> @@ -200,11 +200,21 @@ static void daemonize(char *rundir, char *pidfile)
>  	if (ret == -1)
>  		exit(EXIT_FAILURE);
>  
> -	pid_file_handle = open(pidfile, O_RDWR | O_CREAT, 0600);
> +	pid_file_handle = open(pidfile, O_RDWR | O_CREAT | O_NOFOLLOW, 0600);
>  	if (pid_file_handle == -1) {
>  		/* Couldn't open lock file */
>  		exit(1);
>  	}
> +
> +	{
> +		struct stat st;
> +
> +		if (fstat(pid_file_handle, &st) == -1)
> +			exit(1);
> +
> +		if (!S_ISREG(st.st_mode))
> +			exit(1);
> +	}
>  	/* Try to lock file */
>  #ifdef LOCKF_SUPPORT
>  	if (lockf(pid_file_handle, F_TLOCK, 0) == -1) {
> 
> base-commit: 2e68039281932e6dc37718a1ea7cbb8e2cda42e6
> 

-- 
 i.

[PATCH v2] tools/power/x86/intel-speed-select: Harden daemon pidfile open

[Srinivas Pandruvada]

From: Ali Ahmet MEMIS <dev@unknownbbqr.xyz>

Avoid symlink-based pidfile clobbering by opening the pidfile with
O_NOFOLLOW and validating it with fstat() before locking/writing.

The daemon currently uses a fixed pidfile path under /tmp. A local
unprivileged user can pre-create a symlink at that path and cause a
root-run daemon instance to write into an attacker-chosen file.

Fixes: 7fd786dfbd2c ("tools/power/x86/intel-speed-select: OOB daemon mode")
Signed-off-by: Ali Ahmet MEMIS <dev@unknownbbqr.xyz>
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Cc: stable@kernel.org
---
[Fixed commit message for long lines]

v2
- Added CC to stable

 tools/power/x86/intel-speed-select/isst-daemon.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/tools/power/x86/intel-speed-select/isst-daemon.c b/tools/power/x86/intel-speed-select/isst-daemon.c
index 66df21b2b573..acedb7432849 100644
--- a/tools/power/x86/intel-speed-select/isst-daemon.c
+++ b/tools/power/x86/intel-speed-select/isst-daemon.c
@@ -148,6 +148,7 @@ static void daemonize(char *rundir, char *pidfile)
 {
 	int pid, sid, i;
 	char str[10];
+	struct stat st;
 	struct sigaction sig_actions;
 	sigset_t sig_set;
 	int ret;
@@ -200,11 +201,17 @@ static void daemonize(char *rundir, char *pidfile)
 	if (ret == -1)
 		exit(EXIT_FAILURE);
 
-	pid_file_handle = open(pidfile, O_RDWR | O_CREAT, 0600);
+	pid_file_handle = open(pidfile, O_RDWR | O_CREAT | O_NOFOLLOW, 0600);
 	if (pid_file_handle == -1) {
 		/* Couldn't open lock file */
 		exit(1);
 	}
+
+	if (fstat(pid_file_handle, &st) == -1)
+		exit(1);
+
+	if (!S_ISREG(st.st_mode))
+		exit(1);
 	/* Try to lock file */
 #ifdef LOCKF_SUPPORT
 	if (lockf(pid_file_handle, F_TLOCK, 0) == -1) {
-- 
2.52.0

Re: [PATCH v2] tools/power/x86/intel-speed-select: Harden daemon pidfile open

[srinivas pandruvada]

On Thu, 2026-04-23 at 22:32 +0300, unknownbbqrx wrote:
> 
> From: ali <dev@unknownbbqr.xyz>
> 
> Avoid symlink-based pidfile clobbering by opening the pidfile with
> O_NOFOLLOW and validating it with fstat() before locking/writing.
> 
> The daemon currently uses a fixed pidfile path under /tmp. A local
> unprivileged user can pre-create a symlink at that path and cause a
> root-run daemon instance to write into an attacker-chosen file.
> 
> Signed-off-by: ali <dev@unknownbbqr.xyz>

Thanks for the patch, I am sorry, but I still can't take it in this
state. It's still an anonymous contribution, which is against policy.

> ---
>  tools/power/x86/intel-speed-select/isst-daemon.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/power/x86/intel-speed-select/isst-daemon.c
> b/tools/power/x86/intel-speed-select/isst-daemon.c
> index 66df21b2b..4346b049d 100644
> --- a/tools/power/x86/intel-speed-select/isst-daemon.c
> +++ b/tools/power/x86/intel-speed-select/isst-daemon.c
> @@ -200,11 +200,21 @@ static void daemonize(char *rundir, char
> *pidfile)
>  	if (ret == -1)
>  		exit(EXIT_FAILURE);
>  
> -	pid_file_handle = open(pidfile, O_RDWR | O_CREAT, 0600);
> +	pid_file_handle = open(pidfile, O_RDWR | O_CREAT |
> O_NOFOLLOW, 0600);
>  	if (pid_file_handle == -1) {
>  		/* Couldn't open lock file */
>  		exit(1);
>  	}
> +
> +	{
> +		struct stat st;
> +
> +		if (fstat(pid_file_handle, &st) == -1)
> +			exit(1);
> +
> +		if (!S_ISREG(st.st_mode))
> +			exit(1);
> +	}

Also move struct stat st out at the top of function. So we don't do
unnecessary indentation with { }.


>  	/* Try to lock file */
>  #ifdef LOCKF_SUPPORT
>  	if (lockf(pid_file_handle, F_TLOCK, 0) == -1) {
> 
> base-commit: 2e68039281932e6dc37718a1ea7cbb8e2cda42e6

[PATCH v2] tools/power/x86/intel-speed-select: Harden daemon pidfile open

[unknownbbqrx]

From: ali <dev@unknownbbqr.xyz>

Avoid symlink-based pidfile clobbering by opening the pidfile with
O_NOFOLLOW and validating it with fstat() before locking/writing.

The daemon currently uses a fixed pidfile path under /tmp. A local
unprivileged user can pre-create a symlink at that path and cause a
root-run daemon instance to write into an attacker-chosen file.

Signed-off-by: ali <dev@unknownbbqr.xyz>
---
 tools/power/x86/intel-speed-select/isst-daemon.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/tools/power/x86/intel-speed-select/isst-daemon.c b/tools/power/x86/intel-speed-select/isst-daemon.c
index 66df21b2b..4346b049d 100644
--- a/tools/power/x86/intel-speed-select/isst-daemon.c
+++ b/tools/power/x86/intel-speed-select/isst-daemon.c
@@ -200,11 +200,21 @@ static void daemonize(char *rundir, char *pidfile)
 	if (ret == -1)
 		exit(EXIT_FAILURE);
 
-	pid_file_handle = open(pidfile, O_RDWR | O_CREAT, 0600);
+	pid_file_handle = open(pidfile, O_RDWR | O_CREAT | O_NOFOLLOW, 0600);
 	if (pid_file_handle == -1) {
 		/* Couldn't open lock file */
 		exit(1);
 	}
+
+	{
+		struct stat st;
+
+		if (fstat(pid_file_handle, &st) == -1)
+			exit(1);
+
+		if (!S_ISREG(st.st_mode))
+			exit(1);
+	}
 	/* Try to lock file */
 #ifdef LOCKF_SUPPORT
 	if (lockf(pid_file_handle, F_TLOCK, 0) == -1) {

base-commit: 2e68039281932e6dc37718a1ea7cbb8e2cda42e6
-- 
2.53.0