linux-ext4 - Re: [PATCH] ext4: Convert int to vm_fault

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]

Date:   Wed, 1 Aug 2018 12:22:59 -0400
From:   "Theodore Y. Ts'o" <tytso@....edu>
To:     Matthew Wilcox <willy@...radead.org>
Cc:     Souptick Joarder <jrdr.linux@...il.com>, adilger.kernel@...ger.ca,
        "Darrick J. Wong" <darrick.wong@...cle.com>,
        Jens Axboe <axboe@...nel.dk>,
        Andreas Gruenbacher <agruenba@...hat.com>,
        Eric Biggers <ebiggers@...gle.com>,
        Greg KH <gregkh@...uxfoundation.org>, kemi.wang@...el.com,
        Sabyasachi Gupta <sabyasachi.linux@...il.com>,
        Brajeswar Ghosh <brajeswar.linux@...il.com>,
        linux-ext4@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ext4: Convert int to vm_fault_t type

On Wed, Aug 01, 2018 at 09:13:19AM -0700, Matthew Wilcox wrote:
> On Wed, Aug 01, 2018 at 12:06:18PM -0400, Theodore Y. Ts'o wrote:
> > On Wed, Aug 01, 2018 at 10:38:30AM -0400, Theodore Y. Ts'o wrote:
> > > I'm going to drop the whole ext4 changes for vm_fault_t for this
> > > cycle, and I'll let you try to fix it up properly for the next cycle.
> > 
> > Here's the fixed up commit that I'm going to drop since you plan to be
> > making changes in block_page_mkwrite(), and I don't want us to get out
> > of sync.
> 
> This looks sane to me.

Yeah, it's fine except for the cognitive load to the file system
programmer where block_page_mkwrite() returns an int, and not a
vm_fault_t, and you have use block_page_mkwrite_return() in order to
convert from the negative error code convention to a vm_fault_t.

Souptick has a separate patch out which changes block_page_mkpage() to
return a vm_fault_t.  It's broken in that it clobbers the error return
and doesn't provide a way for the caller to get the error return.  So
Souptick, please consider that other patch to have received a NACK
from me, as it *will* break ext4.

Souptick, perhaps you could change block_page_mkwrite() so that its
function signature looks like this instead:

vm_fault_t block_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf,
	   		      get_block_t get_block, int *err)

...that's sane.  Or you can maybe simply change the *name* of the
function so it's clear it's differnt from all other xxx_page_mkwrite()
functions in that it returns an int.

I'll let you decide what you want to do --- since part of your
development to be a sophisticated programmer is to get experience
making these sorts of decisions that involve having good programming
"taste".

Regards,

						- Ted