linux-kernel - Re: [Intel-gfx] [PATCH 2/2] drm/i915: properly prefault for pread/pwrite

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] [day] [month] [year] [list]

Message-ID: <20111103221024.GG2970@phenom.ffwll.local>
Date:	Thu, 3 Nov 2011 23:10:24 +0100
From:	Daniel Vetter <daniel@...ll.ch>
To:	Keith Packard <keithp@...thp.com>
Cc:	Daniel Vetter <daniel@...ll.ch>,
	Chris Wilson <chris@...is-wilson.co.uk>,
	Daniel Vetter <daniel.vetter@...ll.ch>,
	intel-gfx <intel-gfx@...ts.freedesktop.org>, stable@...nel.org,
	linux-kernel <linux-kernel@...r.kernel.org>,
	"Morton, Andrew" <akpm@...ux-foundation.org>
Subject: Re: [Intel-gfx] [PATCH 2/2] drm/i915: properly prefault for
 pread/pwrite

On Thu, Nov 03, 2011 at 02:06:55PM -0700, Keith Packard wrote:
> On Mon, 24 Oct 2011 00:11:57 +0200, Daniel Vetter <daniel@...ll.ch> wrote:
> 
> > This patch only fixes things up so that we prefault the entire page range
> > and not just the first PAGE_SIZE bytes (i.e. at most 2 pages). So I don't
> > see the risk of extending the current behaviour to all pages. Userspace
> > can already see these zero writes, but only when doing something stupid.
> 
> When we posted a patch to instead fix fault_in_pages_writeable, Andrew
> complained that we'd have modified memory even on a short read, which
> wasn't considered polite. Could we read/write the same value and avoid
> that problem?

Hm, that might be a solution. My current plan was to ditch the prefault
for writing to userspace and beat my pwrite/pread patches into shape for
submission - the bug report only concerns -EFAULT due to handing in a gtt
mapping in pwrite, afaik.

otoh gem objects never change their size and we return -EINVAL if the read
would go past the end of it. And userspace should also never see short
reads due to signals, because the libdrm ioctl automatically restarts the
syscall - and that part is more or less abi. So in practice for our case,
I think it just doesn't matter because userspace really only sees these
zero writes when doing something buggy.

> Also, we should be fixing fault_in_pages_* going forward, rather than
> kludging in more code. And, we'd get to remove the version in ntfs,
> which should end in a patch that removes more code than it adds...

Hm, haven't noticed the version in nfs. The version in pagemap.h does what
all the other users of it want, namely prefault at most PAGE_SIZE bytes
(from at most two pages, in case the user pointer crosses a page boundary).
Which is why I've left it as is.
-Daniel
-- 
Daniel Vetter
Mail: daniel@...ll.ch
Mobile: +41 (0)79 365 57 48
--
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/