All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jason Gunthorpe <jgg@ziepe.ca>
To: Cheng Xu <chengyou@linux.alibaba.com>
Cc: Leon Romanovsky <leon@kernel.org>,
	linux-rdma@vger.kernel.org, KaiShen@linux.alibaba.com,
	Yossi Leybovich <sleybo@amazon.com>,
	Gal Pressman <gal@nvidia.com>
Subject: Re: [PATCH for-next v2 2/2] RDMA/erdma: Support non-4K page size in doorbell allocation
Date: Wed, 22 Mar 2023 11:01:00 -0300	[thread overview]
Message-ID: <ZBsKHBN2NIFA6/YD@ziepe.ca> (raw)
In-Reply-To: <6c982b76-61b2-7317-ab76-8ff0b4fb4471@linux.alibaba.com>

On Wed, Mar 22, 2023 at 09:30:41PM +0800, Cheng Xu wrote:
> 
> 
> On 3/22/23 7:54 PM, Jason Gunthorpe wrote:
> > On Wed, Mar 22, 2023 at 03:05:29PM +0800, Cheng Xu wrote:
> > 
> >> The current generation of erdma devices do not have this capability due to
> >> implementation complexity. Without this HW capability, isolating the MMIO
> >> space in software doesn't prevent the attack, because the malicious APPs
> >> can map mmio itself, not through verbs interface.
> > 
> > This doesn't meet the security model of Linux, verbs HW is expected to
> > protect one process from another process.
> 
> OK, I see.
> 
> So the key point is that HW should restrict each process to use its own doorbell
> space. If hardware can do this, share or do not share MMIO pages both will meet
> the security requirement. Do I get it right? 

HW can never do that, HW is supposed to rely on the system MMU to
isolate doorbell registers

The HW responsibility is to make doorbell MMIO registers safe in the
hands of other processes.

Simple doorbells that only 'kick' and don't convey any information are
probably safe to share, and don't require HW checks between the
doorbell page and the PD/QP/CQ/etc

Doorbells that deliver data - eg a head pointer - are not safe because
the wrong head pointer can corrupt the HW state. Process B must not be
able to corrupt the head pointer of a QP/CQ owned by Process A under
any circumstances. Definitely they cannot have access to the MMIO and
also the HW must ensure that writes coming from process B are rejected
if they touch resources owned by process a (eg by PD/QPN/CQN checks in
HW)

Doorbells that accept entire WQE's are definately not safe as a
hostile process could execute a WQE on a QP it does not own.

> It seems that EFA uses shared MMIO pages with hardware security assurance.

I'm not sure how EFA works, it writes this:

        EFA_SET(&db, EFA_IO_REGS_CQ_DB_CONSUMER_INDEX, cq->cc);
        EFA_SET(&db, EFA_IO_REGS_CQ_DB_CMD_SN, cq->cmd_sn & 0x3);
        EFA_SET(&db, EFA_IO_REGS_CQ_DB_ARM, arm);

But interestingly there is no CQN value, so I have no idea how it
associates the doorbell register with the CQ to load the data into.

If it is using a DB register offset to learn the CQN then obviously it
cannot share the same doorbell page (or associated CQNs) across verbs
FD contexts, that would be a security bug. EFA folks?

Jason

  reply	other threads:[~2023-03-22 14:01 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-07 10:29 [PATCH for-next v2 0/2] RDMA/erdma: Add non-4K page size support Cheng Xu
2023-03-07 10:29 ` [PATCH for-next v2 1/2] RDMA/erdma: Use fixed hardware page size Cheng Xu
2023-03-24 14:34   ` Jason Gunthorpe
2023-03-07 10:29 ` [PATCH for-next v2 2/2] RDMA/erdma: Support non-4K page size in doorbell allocation Cheng Xu
2023-03-14 10:23   ` Leon Romanovsky
     [not found]     ` <5b0cc34d-a185-d9b4-c312-27bc959d929d@linux.alibaba.com>
2023-03-14 11:34       ` Cheng Xu
2023-03-14 11:50     ` Cheng Xu
2023-03-14 14:10       ` Leon Romanovsky
2023-03-15  1:58         ` Cheng Xu
2023-03-15 10:22           ` Leon Romanovsky
2023-03-21 14:30             ` Jason Gunthorpe
2023-03-22  7:05               ` Cheng Xu
2023-03-22 11:54                 ` Jason Gunthorpe
2023-03-22 13:30                   ` Cheng Xu
2023-03-22 14:01                     ` Jason Gunthorpe [this message]
2023-03-22 15:09                       ` Gal Pressman
2023-03-23  6:57                       ` Cheng Xu
2023-03-23 11:53                         ` Jason Gunthorpe
2023-03-23 12:33                           ` Cheng Xu
2023-03-23 13:05                             ` Jason Gunthorpe
2023-03-23 14:10                               ` Cheng Xu
2023-03-23 14:18                                 ` Jason Gunthorpe
2023-03-26  0:10                                   ` Cheng Xu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZBsKHBN2NIFA6/YD@ziepe.ca \
    --to=jgg@ziepe.ca \
    --cc=KaiShen@linux.alibaba.com \
    --cc=chengyou@linux.alibaba.com \
    --cc=gal@nvidia.com \
    --cc=leon@kernel.org \
    --cc=linux-rdma@vger.kernel.org \
    --cc=sleybo@amazon.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.