Re: [PATCH security IB/uverbs: Perform validity check for supplied port number in create_ah

[Leon Romanovsky <leon@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 3181 bytes --]

On Thu, Jun 29, 2017 at 12:41:58PM -0400, Doug Ledford wrote:
> On Tue, 2017-06-27 at 15:09 +0300, Leon Romanovsky wrote:
> >
> > Hi Doug and Security Team,
> >
> > How should we proceed with the following patch?
> >
> > The malicious user (non-root) can send ib_create_ah() comamnd
> > to exposed /sys/class/infiniband_verbs/uverbs* file.
>
> Your explanation is not making a lot of sense.  The
> /sys/class/infiniband_verbs/uverbs* files are directories, not files.
> Are you saying we have an open method by which you can open the
> directory in question and then send ib verbs commands over the
> directory file?  And even if you really mean some other file in that
> directory, why would we be fixing the create_ah handler instead of
> fixing the sysfs write handler for that file to not accept ib verbs
> commands?  Why would we be talking ib verbs commands on *any* sysfs
> file?

You are right,
It was my mistake and the real issue with /dev/infiniband/uverbs0, which
is open for everypne for write:

root@mtr-leonro:/sys/class/infiniband_verbs# ls -al /dev/infiniband/uverbs0
crw-rw-rw- 1 root root 231, 192 Jun 29 14:24 /dev/infiniband/uverbs0


>
> >  All that is
> > needed is to provide port number which is out-of-range and it will
> > kill the system.
>
> Why is this not an issue on the normal /dev/infiniband/uverbs* files
> (which are world writable)?  Is that merely because rdma-
> core/libibverbs checks the port number before submitting the command?
> If so, then a maliciously changed rdma-core/libibverbs would have the
> same problem.

It is exactly the problem. We started to run fuzzy testing on the
ibverbs interface with direct calls to uverbs files without any
libibverbs/rdma-core involvement. For years, we checked our stack as
a bundle (user + kernel), this is why libibvers/rdma-core limited us
to find it before.

>
> > There is need to be root to open uverbs* file, but after that those
> > persmissions can be dropped.
>
> I don't get the issue with the sysfs file, but the bug appears to be
> exploitable by any user who is willing to recompile rdma-core to bypass
> any checks it might have on input validity.

Please take my apology for sysfs. It was a mistake.

You don't need to create special library for that. Boris has
reproduction program, which does open/mmap/write in similar way to
rdma-core.


> From what I can tell, the
> offending code that should be the source of the problem is
> rdma_ah_find_type() which uses the user supplied port_num for an array
> lookup without checking it for validity, thereby being tricked into
> going outside the array bounds by user input.  We call
> rdma_ah_find_type() in two locations: ib_verbs.c:modify_qp() and
> ib_verbs.c:ib_uverbs_create_ah().  Why is this a bug in one and not the
> other?

We don't have fuzzy templates for all commands yet. IMHO, Boris has only
3-4 commands and didn't do modify_qp yet.

> And in modify_qp() it looks like we actually use cmd-
> >base.port_num, cmd->base.alt_port_num, and cmd->base.dest.port_num,
> all as user provided values without checking.

In general, I have 2 more similar bugs pending submissions and would
like to know the procedure.

Thanks

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]