Re: NULL pointer dereference in swsusp_free with 3.17-rc5

["Bjørn Mork" <bjorn@mork.no>]

"Rafael J. Wysocki" <rjw@rjwysocki.net> writes:

> On Tuesday, September 23, 2014 07:27:06 PM Bjørn Mork wrote:
>> 
>> --=-=-=
>> Content-Type: text/plain; charset=utf-8
>> Content-Transfer-Encoding: quoted-printable
>> 
>> "Rafael J. Wysocki" <rjw@rjwysocki.net> writes:
>> 
>> 
>> > I would suspect one of these commits:
>> >
>> > 84c91b7ae07c PM / hibernate: avoid unsafe pages in e820 reserved regions
>> > 0f7d83e85dbd PM / Hibernate: Touch Soft Lockup Watchdog in rtree_next_node
>> > 9047eb629e5c PM / Hibernate: Remove the old memory-bitmap implementation
>> > 6efde38f0769 PM / Hibernate: Iterate over set bits instead of PFNs in sws=
>> usp_free()
>> > 3a20cb177961 PM / Hibernate: Implement position keeping in radix tree
>> > 07a338236fdc PM / Hibernate: Add memory_rtree_find_bit function
>> > f469f02dc6fa PM / Hibernate: Create a Radix-Tree to store memory bitmap
>> >
>> > so I guess you can start from checking them (the topmpost one is the late=
>> st).
>> 
>> Thanks.  Yes, you were correct. The bad commit is
>> 
>>  6efde38f0769 PM / Hibernate: Iterate over set bits instead of PFNs in swsu=
>> sp_free()
>> 
>> I have confirmed that reverting only this commit on top of a clean
>> v3.17-rc6 fixes the problem.  I am attaching the context-modified revert
>> patch I used.
>
> Instead of reverting the commit, can you please check if adding an "if (page)"
> check around 
>
> 		memory_bm_clear_current(forbidden_pages_map);
> 		memory_bm_clear_current(free_pages_map);
> 		__free_page(page);
>
> in swsusp_free() makes the NULL pointer deref go away?

Tested.  But on a hunch, I also added this just to be sure:


@@ -1343,7 +1343,15 @@ void swsusp_free(void)
 {
        unsigned long fb_pfn, fr_pfn;
 
+WARN_ON(!forbidden_pages_map);
+if (!forbidden_pages_map)
+       return;
+
        memory_bm_position_reset(forbidden_pages_map);
+WARN_ON(!free_pages_map);
+if (!free_pages_map)
+       return;
+
        memory_bm_position_reset(free_pages_map);
 



And got:


[   34.500851] WARNING: CPU: 1 PID: 5052 at kernel/power/snapshot.c:1346 swsusp_free+0x25/0x160()
[   34.500854] Modules linked in: xt_multiport iptable_filter ip_tables dm_mod bnep xt_hl nf_log_ipv6 nf_log_common xt_LOG binfmt_misc ip6table_filter ip6_tables x_tables nfsd nfs_acl nfs lockd fscache sunrpc 8021q garp stp mrp llc tun loop fuse snd_hda_codec_conexant snd_hda_codec_generic iTCO_wdt iTCO_vendor_support arc4 coretemp kvm_intel cdc_mbim kvm cdc_wdm uvcvideo cdc_ncm videobuf2_vmalloc evdev videobuf2_memops videobuf2_core psmouse cdc_acm v4l2_common usbnet mii videodev serio_raw i2c_i801 iwlmvm snd_hda_intel snd_hda_controller mac80211 snd_hda_codec snd_hwdep snd_pcm_oss snd_mixer_oss iwlwifi i915 lpc_ich i2c_algo_bit mfd_core drm_kms_helper snd_pcm cfg80211 snd_timer drm wmi thinkpad_acpi nvram snd soundcore battery ecb ac btusb bluetooth video rfkill button acpi_cpufreq processor ext4 crc16 jbd2 mbcache nbd sg sd_mod crc_t10dif sr_mod crct10dif_generic cdrom crct10dif_common microcode ahci libahci libata scsi_mod uhci_hcd ehci_pci ehci_hcd e1000e usbcore ptp usb_common pps_core thermal thermal_sys
[   34.501050] CPU: 1 PID: 5052 Comm: s2disk Not tainted 3.17.0-rc6+ #260
[   34.501054] Hardware name: LENOVO 2776LEG/2776LEG, BIOS 6EET55WW (3.15 ) 12/19/2011
[   34.501057]  0000000000000009 ffff88021e9cfde8 ffffffff813bea17 0000000000000000
[   34.501066]  0000000000000000 ffff88021e9cfe28 ffffffff8103eb60 ffffffff8107f8be
[   34.501075]  ffffffff8107c902 ffff8800ae809a00 0000000000000010 ffff880232149690
[   34.501083] Call Trace:
[   34.501092]  [<ffffffff813bea17>] dump_stack+0x4e/0x68
[   34.501100]  [<ffffffff8103eb60>] warn_slowpath_common+0x7c/0x96
[   34.501107]  [<ffffffff8107f8be>] ? lock_system_sleep+0x22/0x24
[   34.501113]  [<ffffffff8107c902>] ? swsusp_free+0x25/0x160
[   34.501119]  [<ffffffff8103eb8f>] warn_slowpath_null+0x15/0x17
[   34.501125]  [<ffffffff8107c902>] swsusp_free+0x25/0x160
[   34.501131]  [<ffffffff8107fb82>] snapshot_release+0x13/0x68
[   34.501138]  [<ffffffff81136c78>] __fput+0x10d/0x1e5
[   34.501144]  [<ffffffff81136d80>] ____fput+0x9/0xb
[   34.501151]  [<ffffffff81056a51>] task_work_run+0x81/0xb0
[   34.501159]  [<ffffffff810026d8>] do_notify_resume+0x55/0x66
[   34.501166]  [<ffffffff811ee7ce>] ? trace_hardirqs_on_thunk+0x3a/0x3f
[   34.501172]  [<ffffffff813c4a98>] int_signal+0x12/0x17
[   34.501177] ---[ end trace fd197b669f9d81a3 ]---

where 1346 is

bjorn@canardo:/usr/local/src/build-tmp/linux$ sed -ne 1346p kernel/power/snapshot.c
WARN_ON(!forbidden_pages_map);


So it looks like that map for some reason is uninitialized when
swsusp_free() is called after a hibernate->resume cycle.  Is this only
hitting me?  If so, then I *really* wonder what I'm doing out of the
ordinary...



Bjørn