On Sat, Aug 18, 2012 at 10:44:16PM +0000, Nicholas A. Bellinger wrote:
> From: Michael S. Tsirkin <mst@xxxxxxxxxx>
>
> 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
>
> Simplest solution is probably just to make the padding
> explicit.
>
> Reported-by: Michael S. Tsirkin <mst@xxxxxxxxxx>
> Signed-off-by: Michael S. Tsirkin <mst@xxxxxxxxxx>
> Signed-off-by: Nicholas Bellinger <nab@xxxxxxxxxxxxxxx>
One other minor change I would do is verify that
reserved field is set to 0, return -EOPNOTSUPP if not.
This will make sure userspace zeroes out this field,
and also allow future extensions to use this
field with an easy way to test for support.
Not a must, but this is what happens for
the padding field in struct vhost_memory,
so it is better to be consistent I think.
> ---
> drivers/vhost/tcm_vhost.h | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/vhost/tcm_vhost.h b/drivers/vhost/tcm_vhost.h
> index e9d5c02..d9e9355 100644
> --- a/drivers/vhost/tcm_vhost.h
> +++ b/drivers/vhost/tcm_vhost.h
> @@ -93,6 +93,7 @@ struct vhost_scsi_target {
> int abi_version;
> char vhost_wwpn[TRANSPORT_IQN_LEN];
> unsigned short vhost_tpgt;
> + unsigned short reserved;
> };
>
> /* VHOST_SCSI specific defines */
> --
> 1.7.2.5
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
