On Thu, Jul 14, 2016 at 12:00 AM, Rafael J. Wysocki <[email protected]> wrote: > On Wed, Jul 13, 2016 at 11:45 PM, Pavel Machek <[email protected]> wrote: >> On Wed 2016-07-13 22:44:24, Rafael J. Wysocki wrote: >>> On Wed, Jul 13, 2016 at 10:26 PM, Pavel Machek <[email protected]> wrote: >>> > On Wed 2016-07-13 22:04:27, Rafael J. Wysocki wrote: >>> >> On Wed, Jul 13, 2016 at 7:01 PM, Pavel Machek <[email protected]> wrote: >>> >> > Hi! >>> >> > >>> >> >> >>and then swapon the swap device, and do a testing. This should be >>> >> >> >>safer? >>> >> >> >Yeah, that's the way. Read-only root is other option. >>> >> >> > >>> >> >> >>>I guess updating documentation would be welcome from my side, >>> >> >> >>>otherwise it should be ok. >>> >> >> >>OK, I'll update the documents. >>> >> >> >Just add fat warning into the documentation. >>> >> >> OK. >>> >> > >>> >> > Actually... If you could add >>> >> > >>> >> > printk(KERN_ALERT "Hibernation image written. If you have any >>> >> > filesystems mounted read-write and attempt to resume, you'll corrupt >>> >> > your data. To prevent that, remove the hibernation image.\n") >>> >> > >>> >> > ...I guess that would save someone's filesystem. (Yes, very high >>> >> > loglevel. If you attempt to do this from anything else then singleuser >>> >> > or initrd, you are asking for problems, so... lets make sure user sees >>> >> > it.) >>> >> >>> >> Please see the new version of this patch: >>> >> https://patchwork.kernel.org/patch/9226837/ >>> > >>> > New version changes nothing, right? You still need to be sure >>> > filesystems are not mounted r/w. So I would still like to see printk() >>> > with warning. >>> >>> It shouldn't matter how they are mounted, because the contents of >>> persistent storage don't change. >> >> @@ -721,6 +724,9 @@ int hibernate(void) >> atomic_inc(&snapshot_device_available); >> Unlock: >> unlock_system_sleep(); >> + if (snapshot_test) >> + software_resume(); >> + >> return error; >> } >> >> Aha, I see, immediate wakeup here. Makes sense. ... ... >> >> No. >> >> AFAICT, freezer is used in hibernation_snapshot, which means at >> Unlock:, kernel threads are running; software_resume() freezes them >> again, but they had chance to run and potentially corrupt the >> persistent storage... right? > > OK, there is a bug. > > The thawing of user space is potentially dangerous, so in the > "snapshot" test mode hibernate() should just call > free_basic_memory_bitmaps() and from there invoke the code below the > Check_image label in software_resume(), roughly.
Or rather call free_basic_memory_bitmaps() and unlock_device_hotplug(), then do swsusp_check() and invoke the code starting with the "PM: Loading hibernation image.\n" message in software_resume(). Thanks, Rafael
