[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH RFC] tcmu: Introduce qemu-tcmu
From: |
Fam Zheng |
Subject: |
Re: [Qemu-devel] [PATCH RFC] tcmu: Introduce qemu-tcmu |
Date: |
Thu, 20 Oct 2016 22:30:59 +0800 |
User-agent: |
Mutt/1.7.0 (2016-08-17) |
On Thu, 10/20 15:08, Stefan Hajnoczi wrote:
> On Wed, Oct 19, 2016 at 06:08:28PM +0800, Fam Zheng wrote:
> > libtcmu is a Linux library for userspace programs to handle TCMU
> > protocol, which is a SCSI transport between the target_core_user.ko and
> > a userspace backend implementation. The former can be seen as a kernel
> > SCSI Low-Level-Driver that forwards scsi requests to userspace via a
> > UIO ring buffer. By linking to libtcmu, a program can serve as a scsi
> > HBA.
> >
> > This patch adds qemu-tcmu utility that serves as a LIO userspace
> > backend. Apart from how it interacts with the rest of the world, the
> > role of qemu-tcmu is much alike to qemu-nbd: it can export any
> > format/protocol that QEMU supports (in iscsi/loopback instead of NBD),
> > for local or remote access. The main advantage with qemu-tcmu lies in
> > the more flexible command protocol (SCSI) and more flexible iscsi or
> > loopback frontends, the latter of which can theoretically allow zero-copy
> > I/O from local machine, which makes qemu-tcmu potentially possible to
> > serve data in better performance.
> >
> > RFC because only minimal scsi commands are now handled. The plan is to
> > refactor and reuse scsi-disk emulation code. Also there is no test code.
> >
> > Similar to NBD, there will be QMP commands to start built-in targets so
> > that guest images can be exported as well.
> >
> > Usage example script (tested on Fedora 24):
> >
> > set -e
> >
> > # For targetcli integration
> > sudo systemctl start tcmu-runner
> >
> > qemu-img create test-img 1G
> >
> > # libtcmu needs to open UIO, give it access
> > sudo chmod 777 /dev/uio0
>
> What are the security implications of tcmu?
>
> If a corrupt image is able to execute arbitrary code in the qemu-tcmu
> process, does /dev/uio0 or the tcmu shared memory interface allow get
> root or kernel privileges?
I haven't audited the code, but target_core_user.ko should contain the access to
/dev/uioX and make sure there is no security risk regarding buggy or malicious
handlers. Otherwise it's a bug that should be fixed. Andy can correct me if I'm
wrong.
>
> >
> > qemu-tcmu test-img &
> >
> > sleep 1
> >
> > IQN=iqn.2003-01.org.linux-iscsi.lemon.x8664:sn.4939fc29108f
> >
> > # Check that we have initialized correctly
> > sudo targetcli ls | grep -q user:qemu
> >
> > # Create iscsi target
> > sudo targetcli ls | grep -q $IQN || sudo targetcli /iscsi create
> > wwn=$IQN
> > sudo targetcli /iscsi/$IQN/tpg1 set attribute \
> > authentication=0 generate_node_acls=1 demo_mode_write_protect=0 \
> > prod_mode_write_protect=0
> >
> > # Create the qemu-tcmu target
> > sudo targetcli ls | grep -q d0 || \
> > sudo targetcli /backstores/user:qemu create d0 1G @drive
> >
> > # Export it as an iscsi LUN
> > sudo targetcli ls | grep -q lun0 || \
> > sudo targetcli /iscsi/$IQN/tpg1/luns create
> > storage_object=/backstores/user:qemu/d0
> >
> > # Inspect the nodes again
> > sudo targetcli ls
> >
> > # Test that the LIO export works
> > qemu-img info iscsi://127.0.0.1/$IQN/0
> > qemu-io iscsi://127.0.0.1/$IQN/0 \
> > -c 'read -P 1 4k 1k' \
> > -c 'write -P 2 4k 1k' \
> > -c 'read -P 2 4k 1k' \
>
> Users probably want either:
>
> 1. Expose disk image as a loopback SCSI device (/dev/sdb) so that qcow2,
> vmdk, etc files can be accessed from the host.
>
> 2. Expose disk image as over iSCSI for access from other applications or
> machines.
>
> It would be nice to offer user-friendly commands for doing this.
> Manually setting up targetcli and qemu-tcmu looks clunky for ad-hoc
> users. If you only need this feature every once in a while then it's a
> pain to look up and run these commands manually.
Yes, that can be done with a separate helper script in scripts/.
> > +#define TCMU_DEBUG 1
>
> Unused
Will remove together when switching to tracing.
>
> > +
> > +#define DPRINTF(...) do { \
> > + printf("[%s:%04d] ", __FILE__, __LINE__); \
> > + printf(__VA_ARGS__); \
> > +} while (0)
>
> Please use tracing instead. The default tracing backend ("log") prints
> to stderr and is therefore very easy to use.
Yes, will use in a formal version.
>
> > +
> > +typedef struct TCMUExport TCMUExport;
> > +
> > +struct TCMUExport {
> > + BlockBackend *blk;
> > + struct tcmu_device *tcmu_dev;
> > + bool writable;
> > + QLIST_ENTRY(TCMUExport) next;
> > +};
> > +
> > +typedef struct {
> > + struct tcmulib_context *tcmulib_ctx;
> > +} TCMUHandlerState;
> > +
> > +static QLIST_HEAD(, TCMUExport) tcmu_exports =
> > + QLIST_HEAD_INITIALIZER(tcmu_exports);
> > +
> > +static TCMUHandlerState *handler_state;
> > +
> > +#define ASCQ_INVALID_FIELD_IN_CDB 0x2400
>
> Should this go into a scsi header?
Yes, this is ad-hoc, will extract things rom hw/scsi/.
>
> > +
> > +typedef struct {
> > + struct tcmulib_cmd *cmd;
> > + TCMUExport *exp;
> > + QEMUIOVector *qiov;
>
> Where is this freed?
Should free it before this struct, will fix.
Thanks for taking a look!
Fam
Message not available