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:	Wed, 08 Apr 2015 14:19:28 +0200
From:	Yann Droneaud <ydroneaud@...eya.com>
To:	Shachar Raindel <raindel@...lanox.com>
Cc:	"oss-security@...ts.openwall.com" <oss-security@...ts.openwall.com>,
	"<linux-rdma@...r.kernel.org> (linux-rdma@...r.kernel.org)" 
	<linux-rdma@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"stable@...r.kernel.org" <stable@...r.kernel.org>
Subject: Re: CVE-2014-8159 kernel: infiniband: uverbs: unprotected physical
 memory access

Hi,

Le jeudi 02 avril 2015 à 16:34 +0000, Shachar Raindel a écrit :
> > -----Original Message-----
> > From: Yann Droneaud [mailto:ydroneaud@...eya.com]
> > Sent: Thursday, April 02, 2015 6:16 PM
> > Le jeudi 02 avril 2015 à 10:52 +0000, Shachar Raindel a écrit :
> > > > -----Original Message-----
> > > > From: Yann Droneaud [mailto:ydroneaud@...eya.com]
> > > > Sent: Thursday, April 02, 2015 1:05 PM
> > > > Le mercredi 18 mars 2015 à 17:39 +0000, Shachar Raindel a écrit :
> > ...
> > > > > +	/*
> > > > > +	 * If the combination of the addr and size requested for this
> > > > memory
> > > > > +	 * region causes an integer overflow, return error.
> > > > > +	 */
> > > > > +	if ((PAGE_ALIGN(addr + size) <= size) ||
> > > > > +	    (PAGE_ALIGN(addr + size) <= addr))
> > > > > +		return ERR_PTR(-EINVAL);
> > > > > +
> > > >
> > > > Can access_ok() be used here ?
> > > >
> > > >          if (!access_ok(writable ? VERIFY_WRITE : VERIFY_READ,
> > > >                         addr, size))
> > > >                   return ERR_PTR(-EINVAL);
> > > >
> > >
> > > No, this will break the current ODP semantics.
> > >
> > > ODP allows the user to register memory that is not accessible yet.
> > > This is a critical design feature, as it allows avoiding holding
> > > a registration cache. Adding this check will break the behavior,
> > > forcing memory to be all accessible when registering an ODP MR.
> > >
> > 
> > Failed to notice previously, but since this would break ODP, and ODP is
> > only available starting v3.19-rc1, my proposed fix might be applicable
> > for older kernel (if not better).
> > 
> 
> Can you explain how this proposed fix is better than the existing patch?
> Why do we want to push to the stable tree a patch that is not in the
> upstream? There is an existing, tested, patch that is going to the tip
> of the development. It even applies cleanly on every kernel version around.
> 

access_ok() check for overflow *and* that the region is the memory range
for the current process. The later check is not done in your proposed 
fix (but it should not be needed as get_user_pages() will be called 
to validate the whole region for non-ODP memory registration).

Anyway, AFAIK access_ok() won't check for address being not NULL and
size not being 0, and I've noticed your proposed fix also ensure address
is not equal to NULL and, more important, that size is not equal to 0:
before v3.15-rc1 and commit eeb8461e36c9 ("IB: Refactor umem to use
linear SG table"), calling ib_umem_get() with size equal to 0 would 
succeed with any arbitrary address ... who knows what might happen in 
the lowlevel drivers (aka. providers) if they got an umem for a 0-sized
memory region.
This part of the changes was not detailled in your commit message: it's
an issue not related to overflow which is addressed by your patch.

So I agree my proposed patch is no better than yours: I've missed the
0-sized memory region issue and didn't take care of NULL address.

Regards.

-- 
Yann Droneaud
OPTEYA


--
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/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ