ftl: check structure sizes for future ABI compatibility

Signed-off-by: Artur Paszkiewicz <artur.paszkiewicz@intel.com>
Change-Id: Ic32f6fe085d94b00d025b6cab7e5073341169a73
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/13677
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>

include/spdk/ftl.h

@@ -73,6 +73,11 @@ struct ftl_stats {
 
 typedef void (*spdk_ftl_stats_fn)(struct ftl_stats *stats, void *cb_arg);
 
+/*
+ * FTL configuration.
+ *
+ * NOTE: Do not change the layout of this structure. Only add new fields at the end.
+ */
 struct spdk_ftl_conf {
 	/* Device's name */
 	char					*name;
@@ -106,20 +111,40 @@ struct spdk_ftl_conf {
 		uint32_t			chunk_free_target;
 	} nv_cache;
 
+	/* Hole at bytes 0x60 - 0x67. */
+	uint8_t					reserved[4];
+
 	/* Name of base block device (zoned or non-zoned) */
 	char					*base_bdev;
 
 	/* Name of cache block device (must support extended metadata) */
 	char					*cache_bdev;
+
 	/* Enable fast shutdown path */
 	bool					fast_shutdown;
-};
+
+	/* Hole at bytes 0x79 - 0x7f. */
+	uint8_t					reserved2[7];
+
+	/*
+	 * The size of spdk_ftl_conf according to the caller of this library is used for ABI
+	 * compatibility. The library uses this field to know how many fields in this
+	 * structure are valid. And the library will populate any remaining fields with default values.
+	 */
+	size_t					conf_size;
+} __attribute__((packed));
+SPDK_STATIC_ASSERT(sizeof(struct spdk_ftl_conf) == 136, "Incorrect size");
 
 enum spdk_ftl_mode {
 	/* Create new device */
 	SPDK_FTL_MODE_CREATE = (1 << 0),
 };
 
+/*
+ * FTL device attributes.
+ *
+ * NOTE: Do not change the layout of this structure. Only add new fields at the end.
+ */
 struct spdk_ftl_attrs {
 	/* Number of logical blocks */
 	uint64_t			num_blocks;
@@ -172,16 +197,20 @@ int spdk_ftl_dev_free(struct spdk_ftl_dev *dev, spdk_ftl_fn cb, void *cb_arg);
  *
  * \param dev device
  * \param attr Attribute structure to fill
+ * \param attrs_size Must be set to sizeof(struct spdk_ftl_attrs)
  */
-void spdk_ftl_dev_get_attrs(const struct spdk_ftl_dev *dev, struct spdk_ftl_attrs *attr);
+void spdk_ftl_dev_get_attrs(const struct spdk_ftl_dev *dev, struct spdk_ftl_attrs *attr,
+			    size_t attrs_size);
 
 /**
  * Retrieve device’s configuration.
  *
  * \param dev device
  * \param conf FTL configuration structure to fill
+ * \param conf_size Must be set to sizeof(struct spdk_ftl_conf)
  */
-void spdk_ftl_dev_get_conf(const struct spdk_ftl_dev *dev, struct spdk_ftl_conf *conf);
+void spdk_ftl_dev_get_conf(const struct spdk_ftl_dev *dev, struct spdk_ftl_conf *conf,
+			   size_t conf_size);
 
 /**
  * Obtain an I/O channel for the device.
@@ -211,8 +240,9 @@ void spdk_ftl_conf_deinit(struct spdk_ftl_conf *conf);
  * Initialize FTL configuration structure with default values.
  *
  * \param conf FTL configuration to initialize
+ * \param conf_size Must be set to sizeof(struct spdk_ftl_conf)
  */
-void spdk_ftl_get_default_conf(struct spdk_ftl_conf *conf);
+void spdk_ftl_get_default_conf(struct spdk_ftl_conf *conf, size_t conf_size);
 
 /**
  * Submits a read to the specified device.

lib/ftl/ftl_core.c

@@ -300,11 +300,13 @@ ftl_needs_reloc(struct spdk_ftl_dev *dev)
 }
 
 void
-spdk_ftl_dev_get_attrs(const struct spdk_ftl_dev *dev, struct spdk_ftl_attrs *attrs)
+spdk_ftl_dev_get_attrs(const struct spdk_ftl_dev *dev, struct spdk_ftl_attrs *attrs,
+		       size_t attrs_size)
 {
 	attrs->num_blocks = dev->num_lbas;
 	attrs->block_size = FTL_BLOCK_SIZE;
 	attrs->optimum_io_size = dev->xfer_size;
+	/* NOTE: check any new fields in attrs against attrs_size */
 }
 
 static void

lib/ftl/utils/ftl_conf.c

@@ -30,15 +30,23 @@ static const struct spdk_ftl_conf g_default_conf = {
 };
 
 void
-spdk_ftl_get_default_conf(struct spdk_ftl_conf *conf)
+spdk_ftl_get_default_conf(struct spdk_ftl_conf *conf, size_t conf_size)
 {
-	*conf = g_default_conf;
+	assert(conf_size > 0);
+	assert(conf_size <= sizeof(struct spdk_ftl_conf));
+
+	memcpy(conf, &g_default_conf, conf_size);
+	conf->conf_size = conf_size;
 }
 
 void
-spdk_ftl_dev_get_conf(const struct spdk_ftl_dev *dev, struct spdk_ftl_conf *conf)
+spdk_ftl_dev_get_conf(const struct spdk_ftl_dev *dev, struct spdk_ftl_conf *conf, size_t conf_size)
 {
-	*conf = dev->conf;
+	assert(conf_size > 0);
+	assert(conf_size <= sizeof(struct spdk_ftl_conf));
+
+	memcpy(conf, &dev->conf, conf_size);
+	conf->conf_size = conf_size;
 }
 
 int
@@ -49,6 +57,10 @@ spdk_ftl_conf_copy(struct spdk_ftl_conf *dst, const struct spdk_ftl_conf *src)
 	char *base_bdev = NULL;
 	char *cache_bdev = NULL;
 
+	if (!src->conf_size || src->conf_size > sizeof(struct spdk_ftl_conf)) {
+		return -EINVAL;
+	}
+
 	if (src->name) {
 		name = strdup(src->name);
 		if (!name) {
@@ -74,7 +86,8 @@ spdk_ftl_conf_copy(struct spdk_ftl_conf *dst, const struct spdk_ftl_conf *src)
 		}
 	}
 
-	*dst = *src;
+	memcpy(dst, src, src->conf_size);
+
 	dst->name = name;
 	dst->core_mask = core_mask;
 	dst->base_bdev = base_bdev;
@@ -102,6 +115,11 @@ ftl_conf_init_dev(struct spdk_ftl_dev *dev, const struct spdk_ftl_conf *conf)
 {
 	int rc;
 
+	if (!conf->conf_size) {
+		FTL_ERRLOG(dev, "FTL configuration is uninitialized\n");
+		return -EINVAL;
+	}
+
 	if (!conf->name) {
 		FTL_ERRLOG(dev, "No FTL name in configuration\n");
 		return -EINVAL;

module/bdev/ftl/bdev_ftl.c

@@ -198,7 +198,7 @@ bdev_ftl_write_config_json(struct spdk_bdev *bdev, struct spdk_json_write_ctx *w
 	struct spdk_ftl_conf conf;
 	char uuid[SPDK_UUID_STRING_LEN];
 
-	spdk_ftl_dev_get_conf(ftl_bdev->dev, &conf);
+	spdk_ftl_dev_get_conf(ftl_bdev->dev, &conf, sizeof(conf));
 
 	spdk_json_write_object_begin(w);
 
@@ -236,8 +236,8 @@ bdev_ftl_dump_info_json(void *ctx, struct spdk_json_write_ctx *w)
 	struct spdk_ftl_attrs attrs;
 	struct spdk_ftl_conf conf;
 
-	spdk_ftl_dev_get_attrs(ftl_bdev->dev, &attrs);
-	spdk_ftl_dev_get_conf(ftl_bdev->dev, &conf);
+	spdk_ftl_dev_get_attrs(ftl_bdev->dev, &attrs, sizeof(attrs));
+	spdk_ftl_dev_get_conf(ftl_bdev->dev, &conf, sizeof(conf));
 
 	spdk_json_write_named_object_begin(w, "ftl");
 
@@ -303,8 +303,8 @@ bdev_ftl_create_cb(struct spdk_ftl_dev *dev, void *ctx, int status)
 		goto error;
 	}
 
-	spdk_ftl_dev_get_attrs(dev, &attrs);
-	spdk_ftl_dev_get_conf(dev, &conf);
+	spdk_ftl_dev_get_attrs(dev, &attrs, sizeof(attrs));
+	spdk_ftl_dev_get_conf(dev, &conf, sizeof(conf));
 
 	ftl_bdev->dev = dev;
 	ftl_bdev->bdev.product_name = "FTL disk";

module/bdev/ftl/bdev_ftl_rpc.c

@@ -82,7 +82,7 @@ rpc_bdev_ftl_create(struct spdk_jsonrpc_request *request,
 	struct spdk_json_write_ctx *w;
 	int rc;
 
-	spdk_ftl_get_default_conf(&conf);
+	spdk_ftl_get_default_conf(&conf, sizeof(conf));
 
 	if (spdk_json_decode_object(params, rpc_bdev_ftl_create_decoders,
 				    SPDK_COUNTOF(rpc_bdev_ftl_create_decoders),