lkml.org 
[lkml]   [2022]   [Jul]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH v3] PM: hibernate: defer device probing when resuming from hibernation
On Fri, Jul 15, 2022 at 7:51 AM Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> wrote:
>
> syzbot is reporting hung task at misc_open() [1], for there is a race
> window of AB-BA deadlock which involves probe_count variable. Currently
> wait_for_device_probe() from snapshot_open() from misc_open() can sleep
> forever with misc_mtx held if probe_count cannot become 0.
>
> When a device is probed by hub_event() work function, probe_count is
> incremented before the probe function starts, and probe_count is
> decremented after the probe function completed.
>
> There are three cases that can prevent probe_count from dropping to 0.
>
> (a) A device being probed stopped responding (i.e. broken/malicious
> hardware).
>
> (b) A process emulating a USB device using /dev/raw-gadget interface
> stopped responding for some reason.
>
> (c) New device probe requests keeps coming in before existing device
> probe requests complete.
>
> The phenomenon syzbot is reporting is (b). A process which is holding
> system_transition_mutex and misc_mtx is waiting for probe_count to become
> 0 inside wait_for_device_probe(), but the probe function which is called
> from hub_event() work function is waiting for the processes which are
> blocked at mutex_lock(&misc_mtx) to respond via /dev/raw-gadget interface.
>
> This patch mitigates (b) by deferring wait_for_device_probe() from
> snapshot_open() to snapshot_write() and snapshot_ioctl(). Please note that
> the possibility of (b) remains as long as any thread which is emulating a
> USB device via /dev/raw-gadget interface can be blocked by uninterruptible
> blocking operations (e.g. mutex_lock()).
>
> Please also note that (a) and (c) are not addressed. Regarding (c), we
> should change the code to wait for only one device which contains the
> image for resuming from hibernation. I don't know how to address (a), for
> use of timeout for wait_for_device_probe() might result in loss of user
> data in the image. Maybe we should require the userland to wait for the
> image device before opening /dev/snapshot interface.
>
> Link: https://syzkaller.appspot.com/bug?extid=358c9ab4c93da7b7238c [1]
> Reported-by: syzbot <syzbot+358c9ab4c93da7b7238c@syzkaller.appspotmail.com>
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Tested-by: syzbot <syzbot+358c9ab4c93da7b7238c@syzkaller.appspotmail.com>
> Cc: Greg KH <gregkh@linuxfoundation.org>
> Cc: Oliver Neukum <oneukum@suse.com>
> Cc: Wedson Almeida Filho <wedsonaf@google.com>
> Cc: Rafael J. Wysocki <rjw@sisk.pl>
> Cc: Arjan van de Ven <arjan@linux.intel.com>
> ---
> Changes in v3:
> Defer wait_for_device_probe() till first write()/ioctl() request.
>
> v2 is at https://lkml.kernel.org/r/03096156-3478-db03-c015-28643479116c@I-love.SAKURA.ne.jp .
>
> v1 is at https://lkml.kernel.org/r/72e74af9-f1b6-e383-a2c3-6ee8a0aea5e0@I-love.SAKURA.ne.jp .
>
> kernel/power/user.c | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/power/user.c b/kernel/power/user.c
> index ad241b4ff64c..d43c2aa583b2 100644
> --- a/kernel/power/user.c
> +++ b/kernel/power/user.c
> @@ -26,6 +26,7 @@
>
> #include "power.h"
>
> +static bool need_wait;
>
> static struct snapshot_data {
> struct snapshot_handle handle;
> @@ -78,7 +79,7 @@ static int snapshot_open(struct inode *inode, struct file *filp)
> * Resuming. We may need to wait for the image device to
> * appear.
> */
> - wait_for_device_probe();
> + need_wait = true;
>
> data->swap = -1;
> data->mode = O_WRONLY;
> @@ -168,6 +169,11 @@ static ssize_t snapshot_write(struct file *filp, const char __user *buf,
> ssize_t res;
> loff_t pg_offp = *offp & ~PAGE_MASK;
>
> + if (need_wait) {
> + wait_for_device_probe();
> + need_wait = false;
> + }
> +
> lock_system_sleep();
>
> data = filp->private_data;
> @@ -244,6 +250,11 @@ static long snapshot_ioctl(struct file *filp, unsigned int cmd,
> loff_t size;
> sector_t offset;
>
> + if (need_wait) {
> + wait_for_device_probe();
> + need_wait = false;
> + }
> +
> if (_IOC_TYPE(cmd) != SNAPSHOT_IOC_MAGIC)
> return -ENOTTY;
> if (_IOC_NR(cmd) > SNAPSHOT_IOC_MAXNR)
> --

Applied as 5.20 material, thanks!

\
 
 \ /
  Last update: 2022-07-26 20:43    [W:0.034 / U:0.064 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site