linux-kernel - Re: [PATCH v4 2/2] mm/gup/writeback: add callbacks for inaccessible pages

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:   Mon, 13 Apr 2020 13:22:24 -0700
From:   Dave Hansen <dave.hansen@...el.com>
To:     Claudio Imbrenda <imbrenda@...ux.ibm.com>,
        linux-next@...r.kernel.org, akpm@...ux-foundation.org,
        jack@...e.cz, kirill@...temov.name
Cc:     borntraeger@...ibm.com, david@...hat.com, aarcange@...hat.com,
        linux-mm@...ck.org, frankja@...ux.ibm.com, sfr@...b.auug.org.au,
        jhubbard@...dia.com, linux-kernel@...r.kernel.org,
        linux-s390@...r.kernel.org, Will Deacon <will@...nel.org>,
        Sean Christopherson <sean.j.christopherson@...el.com>
Subject: Re: [PATCH v4 2/2] mm/gup/writeback: add callbacks for inaccessible
 pages

On 3/6/20 5:25 AM, Claudio Imbrenda wrote:
> On s390x the function is not supposed to fail, so it is ok to use a
> WARN_ON on failure. If we ever need some more finegrained handling
> we can tackle this when we know the details.

Could you explain a bit why the function can't fail?

If the guest has secret data in the page, then it *can* and does fail.
It won't fail, though, if the host and guest agree on whether the page
is protected.

Right?

> @@ -2807,6 +2807,13 @@ int __test_set_page_writeback(struct page *page, bool keep_write)
>  		inc_zone_page_state(page, NR_ZONE_WRITE_PENDING);
>  	}
>  	unlock_page_memcg(page);
> +	access_ret = arch_make_page_accessible(page);
> +	/*
> +	 * If writeback has been triggered on a page that cannot be made
> +	 * accessible, it is too late to recover here.
> +	 */
> +	VM_BUG_ON_PAGE(access_ret != 0, page);
> +
>  	return ret;
>  
>  }

This seems like a really odd place to do this.  Writeback is specific to
block I/O.  I would have thought there were other kinds of devices that
matter, not just block devices.

Also, this patch seems odd that it only does the
arch_make_page_accessible() half.  Where's the other half where the page
is made inaccessible?

I assume it's OK to "leak" things like this, it's just not clear to me
_why_ it's OK.