On Sat, 2012-08-18 at 23:04 +0300, Michael S. Tsirkin wrote:
> Hi Nicholas,
> I just noticed this problem in the interface:
>
> +#include <linux/vhost.h>
> +
> +/*
> + * Used by QEMU userspace to ensure a consistent vhost-scsi ABI.
> + *
> + * ABI Rev 0: July 2012 version starting point for v3.6-rc merge
> candidate +
> + * RFC-v2 vhost-scsi userspace. Add GET_ABI_VERSION ioctl
> usage
> + */
> +
> +#define VHOST_SCSI_ABI_VERSION 0
> +
> +struct vhost_scsi_target {
> + int abi_version;
> + unsigned char vhost_wwpn[TRANSPORT_IQN_LEN];
> + unsigned short vhost_tpgt;
> +};
> +
>
> Here TRANSPORT_IQN_LEN is 224, which is a multiple of 4.
> Since vhost_tpgt is 2 bytes and abi_version is 4, the total size would
> be 230. But gcc needs struct size be aligned to first field size, which
> is 4 bytes, so it pads the structure by extra 2 bytes to the total of
> 232.
>
> This padding is very undesirable in an ABI:
> - it can not be initialized easily
> - it can not be checked easily
> - it can leak information between kernel and userspace
>
Hmmmm, yes. Very good reasons to avoid ABI ambiguity ..
> Simplest solution is probably just to make the padding
> explicit:
>
> +struct vhost_scsi_target {
> + int abi_version;
> + unsigned char vhost_wwpn[TRANSPORT_IQN_LEN];
> + unsigned short vhost_tpgt;
> + unsigned short reserved;
> +};
> +
>
> I think we should fix this buglet before it goes out to users.
>
<nod>, fixing this up in target-pending/master now w/ your reported-by
+signoff, and will change vhost-scsi's copy of these defs for next
week's RFC-v3 posting.
Thanks MST!
--nab
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
