linux-kernel - Re: [PATCH v2 4/7] iov_iter: new iov_iter_pin

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <YxhbQ/oVMvt3nG9i@infradead.org>
Date:   Wed, 7 Sep 2022 01:50:11 -0700
From:   Christoph Hellwig <hch@...radead.org>
To:     John Hubbard <jhubbard@...dia.com>
Cc:     Christoph Hellwig <hch@...radead.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Jens Axboe <axboe@...nel.dk>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        Miklos Szeredi <miklos@...redi.hu>,
        "Darrick J . Wong" <djwong@...nel.org>,
        Trond Myklebust <trond.myklebust@...merspace.com>,
        Anna Schumaker <anna@...nel.org>, Jan Kara <jack@...e.cz>,
        David Hildenbrand <david@...hat.com>,
        Logan Gunthorpe <logang@...tatee.com>,
        linux-block@...r.kernel.org, linux-fsdevel@...r.kernel.org,
        linux-xfs@...r.kernel.org, linux-nfs@...r.kernel.org,
        linux-mm@...ck.org, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 4/7] iov_iter: new iov_iter_pin_pages*() routines

On Tue, Sep 06, 2022 at 12:58:59AM -0700, John Hubbard wrote:
> > For FOLL_PIN callers, never pin bvec and kvec pages:  For file systems
> > not acquiring a reference is obviously safe, and the other callers will
> > need an audit, but I can't think of why it woul  ever be unsafe.
> 
> In order to do that, one would need to be confident that such bvec and kvec
> pages do not get passed down to bio_release_pages() (or the new
> bio_unpin_pages()). Also, I'm missing a key point, because today bvec pages get
> a get_page() reference from __iov_iter_get_pages_alloc(). If I just skip
> that, then the get/put calls are unbalanced...

Except that for the most relevant callers (bdev and iomap) we never
end up in __iov_iter_get_pages_alloc. What I suggest is that, with
the move to the pin helper, we codify that logic.

For callers of the bio_iov_iter_pin_pages helper, the corresponding
bio_unpin_pages helper will encapsulate that logic.  For direct calls
to iov_iter_pin_pages, we should add a iov_iter_unpin_pages helper that
also encapsulates the logic.  The beauty is that this means the caller
itself does not have to care about any of thise, and the logic is in
one (well two due to the block special case that reuses the bio_vec
array for the pages space) place.

> I can just hear Al Viro repeating his points about splice() and vmsplice(),
> heh. :)

splice does take these references before calling into the file system
(see my reply to Jan).