Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Date:   Tue, 17 May 2022 15:06:34 +0100
From:   Stefan Hajnoczi <stefanha@redhat.com>
To:     Ming Lei <ming.lei@redhat.com>
Cc:     Jens Axboe <axboe@kernel.dk>, linux-block@vger.kernel.org,
        linux-kernel@vger.kernel.org,
        Harris James R <james.r.harris@intel.com>,
        io-uring@vger.kernel.org,
        Gabriel Krisman Bertazi <krisman@collabora.com>,
        ZiyangZhang <ZiyangZhang@linux.alibaba.com>,
        Xiaoguang Wang <xiaoguang.wang@linux.alibaba.com>
Subject: Re: [PATCH V2 0/1] ubd: add io_uring based userspace block driver
Message-ID: <YoOr6jBfgVm8GvWg@stefanha-x1.localdomain>
References: <20220517055358.3164431-1-ming.lei@redhat.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="uCYJ7l6rM0lflhAj"
Content-Disposition: inline
In-Reply-To: <20220517055358.3164431-1-ming.lei@redhat.com>
Precedence: bulk


--uCYJ7l6rM0lflhAj
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Here are some more thoughts on the ubd-control device:

The current patch provides a ubd-control device for processes with
suitable permissions (i.e. root) to create, start, stop, and fetch
information about devices.

There is no isolation between devices created by one process and those
created by another. Therefore two processes that do not trust each other
cannot both use UBD without potential interference. There is also no
isolation for containers.

I think it would be a mistake to keep the ubd-control interface in its
current form since the current global/root model is limited. Instead I
suggest:
- Creating a device returns a new file descriptor instead of a global
  dev_id. The device can be started/stopped/configured through this (and
  only through this) per-device file descriptor. The device is not
  visible to other processes through ubd-control so interference is not
  possible. In order to give another process control over the device the
  fd can be passed (e.g. SCM_RIGHTS).=20

Now multiple applications/containers/etc can use ubd-control without
interfering with each other. The security model still requires root
though since devices can be malicious.

FUSE allows unprivileged mounts (see fuse_allow_current_process()). Only
processes with the same uid as the FUSE daemon can access such mounts
(in the default configuration). This prevents security issues while
still allowing unprivileged use cases.

I suggest adapting the FUSE security model to block devices:
- Devices can be created without CAP_SYS_ADMIN but they have an
  'unprivileged' flag set to true.
- Unprivileged devices are not probed for partitions and LVM doesn't
  touch them. This means the kernel doesn't access these devices via
  code paths that might be exploitable.
- When another process with a different uid from ubdsrv opens an
  unprivileged device, -EACCES is returned. This protects other
  uids from the unprivileged device.
- When another process with a different uid from ubdsrv opens a
  _privileged_ device there is no special access check because ubdsrv is
  privileged.

With these changes UBD can be used by unprivileged processes and
containers. I think it's worth discussing the details and having this
model from the start so UBD can be used in a wide range of use cases.

Stefan

--uCYJ7l6rM0lflhAj
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQEzBAEBCAAdFiEEhpWov9P5fNqsNXdanKSrs4Grc8gFAmKDq+oACgkQnKSrs4Gr
c8gciAf+JxH0nZCAHY7X+muFxCm6VDyKoarHuOh9NAWV2JRk5Bb12LHTTfl0+1yn
UeZAQuRg7ez0Ur8CXPkc+5FCgBR/Ahqm7iThJ6tns+mErvKkducPXLiLXudZn11o
mdFgAI8bi2W/REoCKLYAweBWHLm2WnKVsL/wVfCDnpXWjE6HCVsFlYmQlBCWN0wD
HolEityrNvgAQeW/hVYV/2Lo6/OVBiLqU6gxMrHUvWlj0WMoLhkhLA4FIDNxs04i
RJYZODQv9jr+tDjYZ+s1ZN8H8AnKkRVTCcMpg76ADhuNMUQjQINGdgSty7uixy4a
BrIMVdd/t/ldnse1423hFtYsqmKnPg==
=OqHO
-----END PGP SIGNATURE-----

--uCYJ7l6rM0lflhAj--