linux-kernel - Re: [PATCH 2/2] Drivers: hv: balloon: Fix the deadlock issue in the memory hot-add code

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20141209105532.GB11373@dhcp22.suse.cz>
Date:	Tue, 9 Dec 2014 11:55:32 +0100
From:	Michal Hocko <mhocko@...e.cz>
To:	Yasuaki Ishimatsu <isimatu.yasuaki@...fujitsu.com>
Cc:	"K. Y. Srinivasan" <kys@...rosoft.com>, gregkh@...uxfoundation.org,
	linux-kernel@...r.kernel.org, devel@...uxdriverproject.org,
	olaf@...fle.de, apw@...onical.com, linux-mm@...ck.org
Subject: Re: [PATCH 2/2] Drivers: hv: balloon: Fix the deadlock issue in the
 memory hot-add code

On Tue 09-12-14 19:25:50, Yasuaki Ishimatsu wrote:
> (2014/12/09 18:08), Michal Hocko wrote:
[...]
> >Doesn't udev retry the operation if it gets EBUSY or EAGAIN?
> 
> It depend on implementation of udev.rules. So we can retry online/offline
> operation in udev.rules.
[...]

# Memory hotadd request
SUBSYSTEM=="memory", ACTION=="add", DEVPATH=="/devices/system/memory/memory*[0-9]", TEST=="/sys$devpath/state", RUN+="/bin/sh -c 'echo online > /sys$devpath/state'"

OK so this is not prepared for a temporary failures and retries.

> >And again, why cannot we simply make the onlining fail or try_lock and
> >retry internally if the event consumer cannot cope with errors?
> 
> Did you mean the following Srinivasan's first patch looks good to you?
>   https://lkml.org/lkml/2014/12/2/662

Heh, I was just about to post this. Because I haven't noticed the
previous patch yet. Yeah, Something like that. Except that I would
expect EAGAIN or EBUSY rather than ERESTARTSYS which should never leak
into userspace. And that would happen here AFAICS because signal_pending
will not be true usually.

So there are two options. Either make the udev rule more robust and
retry within RUN section or do the retry withing online_pages (try_lock
and go into interruptible sleep which gets signaled by finished
add_memory()). The later option is safer wrt. the userspace because the
operation wouldn't fail unexpectedly.
Another option would be generating the sysfs file after all the internal
initialization is done and call it outside of the memory hotplug lock.

-- 
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/