linux-kernel - Re: [PATCH 9/9] iomap: Change calling convention for zeroing

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]

Date:   Tue, 25 Aug 2020 04:26:03 +0100
From:   Matthew Wilcox <willy@...radead.org>
To:     Dave Chinner <david@...morbit.com>
Cc:     linux-xfs@...r.kernel.org, linux-fsdevel@...r.kernel.org,
        "Darrick J . Wong" <darrick.wong@...cle.com>,
        linux-nvdimm@...ts.01.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 9/9] iomap: Change calling convention for zeroing

On Tue, Aug 25, 2020 at 10:27:35AM +1000, Dave Chinner wrote:
> >  	do {
> > -		unsigned offset, bytes;
> > -
> > -		offset = offset_in_page(pos);
> > -		bytes = min_t(loff_t, PAGE_SIZE - offset, count);
> > +		loff_t bytes;
> >  
> >  		if (IS_DAX(inode))
> > -			status = dax_iomap_zero(pos, offset, bytes, iomap);
> > +			bytes = dax_iomap_zero(pos, length, iomap);
> 
> Hmmm. everything is loff_t here, but the callers are defining length
> as u64, not loff_t. Is there a potential sign conversion problem
> here? (sure 64 bit is way beyond anything we'll pass here, but...)

I've gone back and forth on the correct type for 'length' a few times.
size_t is too small (not for zeroing, but for seek()).  An unsigned type
seems right -- a length can't be negative, and we don't want to give
the impression that it can.  But the return value from these functions
definitely needs to be signed so we can represent an error.  So a u64
length with an loff_t return type feels like the best solution.  And
the upper layers have to promise not to pass in a length that's more
than 2^63-1.

I have a patch set which starts the conversion process for all the actors
over to using u64 for the length.  Obviously, that's not mixed in with
the THP patchset.