linux-kernel - Re: [PATCH v11 02/21] block: Change direct

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: <20141016193921.GC11522@wil.cx>
Date:	Thu, 16 Oct 2014 15:39:21 -0400
From:	Matthew Wilcox <willy@...ux.intel.com>
To:	Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
Cc:	Matthew Wilcox <matthew.r.wilcox@...el.com>,
	linux-fsdevel@...r.kernel.org, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v11 02/21] block: Change direct_access calling convention

On Thu, Oct 16, 2014 at 10:45:50AM +0200, Mathieu Desnoyers wrote:
> > -static int
> > +static long
> >  axon_ram_direct_access(struct block_device *device, sector_t sector,
> > -		       void **kaddr, unsigned long *pfn)
> > +		       void **kaddr, unsigned long *pfn, long size)
> 
> Why "long" as type for size ? What is the intent to have it signed, and
> why using a 32-bit type on 32-bit architectures rather than 64-bit ?
> Can we run into issues if we try to map a >2GB file on 32-bit
> architectures ?

The interface requires that the entirety of the pmem be mapped at
all times (see the void **kaddr).  So the total amount of pmem in the
system can't be larger than 4GB on a 32-bit system.  On x86-32, that's
actually limited to 1GB (because we give userspace 3GB), so the problem
doesn't come up.  Maybe this would be more of a potetial problem on
other architectures.

As noted elsewhere in the thread, it would be possible, and maybe
desirable, to remove the need to have all of pmem mapped into the kernel
address space at all times, but I'm not looking to solve that problem
with this patch-set.

The intent of having it signed is that users pass in the size they want
to have and are returned the size they actually got.  Since the function
must be able to return an error, keeping size signed is natural.

> > +long bdev_direct_access(struct block_device *bdev, sector_t sector,
> > +			void **addr, unsigned long *pfn, long size)
> > +{
> > +	long avail;
> > +	const struct block_device_operations *ops = bdev->bd_disk->fops;
> > +
> > +	if (size < 0)
> > +		return size;
> 
> I'm wondering how we should handle size == 0 here. Should it be accepted
> or refused ?

It is a bit of a bizarre case.  I'm inclined to the current behaviour
of saying "this is the address where you can access zero bytes" :-)

But maybe it indicates a bug in the caller, and being noisy about it
would result in the caller getting fixed.
--
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/