* Unsafe find_vpid() calls
@ 2024-11-28 14:58 Christian Brauner
2024-11-28 15:07 ` Greg Kroah-Hartman
0 siblings, 1 reply; 2+ messages in thread
From: Christian Brauner @ 2024-11-28 14:58 UTC (permalink / raw)
To: Scott Branden, Greg Kroah-Hartman, Hans de Goede, Larry Finger
Cc: linux-fsdevel, linux-kernel, Broadcom internal kernel review list
Hey,
You have various calls to find_vpid() in your drivers that aren't
protected by either tasklist_lock or rcu_read_lock(). Afair, this is
unsafe as the struct pid might be freed beneath you. You should please
fix those places to be protected by rcu_read_lock(). Something like the
below or similar should work.
Thanks!
Christian
diff --git a/drivers/misc/bcm-vk/bcm_vk_dev.c b/drivers/misc/bcm-vk/bcm_vk_dev.c
index d4a96137728d..84cab909db71 100644
--- a/drivers/misc/bcm-vk/bcm_vk_dev.c
+++ b/drivers/misc/bcm-vk/bcm_vk_dev.c
@@ -522,7 +522,9 @@ void bcm_vk_blk_drv_access(struct bcm_vk *vk)
dev_dbg(&vk->pdev->dev,
"Send kill signal to pid %d\n",
ctx->pid);
+ rcu_read_lock();
kill_pid(find_vpid(ctx->pid), SIGKILL, 1);
+ rcu_read_unlock();
}
}
}
diff --git a/drivers/misc/bcm-vk/bcm_vk_tty.c b/drivers/misc/bcm-vk/bcm_vk_tty.c
index 59bab76ff0a9..6bd93347938e 100644
--- a/drivers/misc/bcm-vk/bcm_vk_tty.c
+++ b/drivers/misc/bcm-vk/bcm_vk_tty.c
@@ -326,8 +326,11 @@ void bcm_vk_tty_terminate_tty_user(struct bcm_vk *vk)
for (i = 0; i < BCM_VK_NUM_TTY; ++i) {
vktty = &vk->tty[i];
- if (vktty->pid)
+ if (vktty->pid) {
+ rcu_read_lock();
kill_pid(find_vpid(vktty->pid), SIGKILL, 1);
+ rcu_read_unlock();
+ }
}
}
diff --git a/drivers/staging/rtl8712/rtl8712_cmd.c b/drivers/staging/rtl8712/rtl8712_cmd.c
index bb7db96ed821..de13f4eab60f 100644
--- a/drivers/staging/rtl8712/rtl8712_cmd.c
+++ b/drivers/staging/rtl8712/rtl8712_cmd.c
@@ -61,7 +61,9 @@ static void check_hw_pbc(struct _adapter *padapter)
*/
if (padapter->pid == 0)
return;
+ rcu_read_lock();
kill_pid(find_vpid(padapter->pid), SIGUSR1, 1);
+ rcu_read_unlock();
}
}
^ permalink raw reply [flat|nested] 2+ messages in thread* Re: Unsafe find_vpid() calls
2024-11-28 14:58 Unsafe find_vpid() calls Christian Brauner
@ 2024-11-28 15:07 ` Greg Kroah-Hartman
0 siblings, 0 replies; 2+ messages in thread
From: Greg Kroah-Hartman @ 2024-11-28 15:07 UTC (permalink / raw)
To: Christian Brauner
Cc: Scott Branden, Hans de Goede, Larry Finger, linux-fsdevel,
linux-kernel, Broadcom internal kernel review list
On Thu, Nov 28, 2024 at 03:58:06PM +0100, Christian Brauner wrote:
> Hey,
>
> You have various calls to find_vpid() in your drivers that aren't
> protected by either tasklist_lock or rcu_read_lock(). Afair, this is
> unsafe as the struct pid might be freed beneath you. You should please
> fix those places to be protected by rcu_read_lock(). Something like the
> below or similar should work.
>
> Thanks!
> Christian
>
> diff --git a/drivers/misc/bcm-vk/bcm_vk_dev.c b/drivers/misc/bcm-vk/bcm_vk_dev.c
> index d4a96137728d..84cab909db71 100644
> --- a/drivers/misc/bcm-vk/bcm_vk_dev.c
> +++ b/drivers/misc/bcm-vk/bcm_vk_dev.c
> @@ -522,7 +522,9 @@ void bcm_vk_blk_drv_access(struct bcm_vk *vk)
> dev_dbg(&vk->pdev->dev,
> "Send kill signal to pid %d\n",
> ctx->pid);
> + rcu_read_lock();
> kill_pid(find_vpid(ctx->pid), SIGKILL, 1);
> + rcu_read_unlock();
> }
> }
> }
> diff --git a/drivers/misc/bcm-vk/bcm_vk_tty.c b/drivers/misc/bcm-vk/bcm_vk_tty.c
> index 59bab76ff0a9..6bd93347938e 100644
> --- a/drivers/misc/bcm-vk/bcm_vk_tty.c
> +++ b/drivers/misc/bcm-vk/bcm_vk_tty.c
> @@ -326,8 +326,11 @@ void bcm_vk_tty_terminate_tty_user(struct bcm_vk *vk)
>
> for (i = 0; i < BCM_VK_NUM_TTY; ++i) {
> vktty = &vk->tty[i];
> - if (vktty->pid)
> + if (vktty->pid) {
> + rcu_read_lock();
> kill_pid(find_vpid(vktty->pid), SIGKILL, 1);
> + rcu_read_unlock();
> + }
> }
> }
>
> diff --git a/drivers/staging/rtl8712/rtl8712_cmd.c b/drivers/staging/rtl8712/rtl8712_cmd.c
> index bb7db96ed821..de13f4eab60f 100644
> --- a/drivers/staging/rtl8712/rtl8712_cmd.c
> +++ b/drivers/staging/rtl8712/rtl8712_cmd.c
> @@ -61,7 +61,9 @@ static void check_hw_pbc(struct _adapter *padapter)
> */
> if (padapter->pid == 0)
> return;
> + rcu_read_lock();
> kill_pid(find_vpid(padapter->pid), SIGUSR1, 1);
> + rcu_read_unlock();
> }
> }
Odds are all of these usages can just be removed entirely, I'll add it
to the "todo" list of mine...
thanks!
greg k-h
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2024-11-28 15:07 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-11-28 14:58 Unsafe find_vpid() calls Christian Brauner
2024-11-28 15:07 ` Greg Kroah-Hartman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®