[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date: Wed, 08 Apr 2015 14:44:03 +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 mercredi 08 avril 2015 à 14:19 +0200, Yann Droneaud a écrit :
> 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
It only check address not being 0 if size is already PAGE_SIZE aligned,
and it only check size not being 0 if address is already PAGE_SIZE
aligned.
> 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
Powered by blists - more mailing lists
Please check out the
Open Source Software Security Wiki, which is counterpart to this
mailing list.
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
