linux-kernel - RE: [RFC PATCH] mm, hwpoison: Recover from copy-on-write machine checks

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]

Date:   Tue, 18 Oct 2022 17:52:05 +0000
From:   "Luck, Tony" <tony.luck@...el.com>
To:     HORIGUCHI NAOYA(堀口　直也) 
        <naoya.horiguchi@....com>
CC:     Andrew Morton <akpm@...ux-foundation.org>,
        Miaohe Lin <linmiaohe@...wei.com>,
        Matthew Wilcox <willy@...radead.org>,
        Shuai Xue <xueshuai@...ux.alibaba.com>,
        "Williams, Dan J" <dan.j.williams@...el.com>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [RFC PATCH] mm, hwpoison: Recover from copy-on-write machine
 checks

>> + *	-1 = copy failed due to poison in source page

> Simply calling "poison" might cause confusion with page poisoning feature,
> so "hwpoison" might be better.  But I know that "poison" is commonly used
> under arch/x86, and it's not clear to me what to do with this terminology.
> Maybe using -EHWPOISON instead of -1 might be helpful to the distinction.

Agreed. Using -EHWPOISON return is clearer here.

>> -		if (!__wp_page_copy_user(new_page, old_page, vmf)) {
>> +		ret = __wp_page_copy_user(new_page, old_page, vmf);
>> +		if (ret == -1) {
>> +			put_page(new_page);
>
> Maybe I miss something, but don't you have to care about refcount of
> old_page in this branch (as done in "ret == 0" branch)?

You didn't miss anything. But I did. More needs to be done with old_page
(it is where the poison is). I got "lucky" just ignoring/forgetting about it in
my patch ... the system just happened to recover, but I think the poison
may not have been handled for the parent process. that still has the page
mapped.. Need to think about this more.

Thanks for the review.

-Tony