Commit 69a8877e authored by Tomasz Zawadzki's avatar Tomasz Zawadzki Committed by Darek Stojaczyk
Browse files

lib/blob: do not allow xattr to exceed maximum descriptor length 



Length of xattr descriptor is equal to length of xattr struct,
xattr name and the len of stored value.

There is no limit to how much can be stored in memory for xattr.
On disk xattr size is limited to single page and within that to
max descriptors that can fit in it.
This size is known at compile time.

Before this patch it was possible to add xattr exceeding
what was possible to be written to disk. This caused issues
when serializing the metadata during spdk_blob_sync_md()
or spdk_blob_close(). Making those fail without specific info
to the user and not actually writting such descriptor.

Since maximum length of xattr descriptor is known at compile time,
this patch compares against this value when setting the xattr.
It will immediately report back to user with error, and will
not store xattr in memory (thus not serialize it).

This patch should not affect any backward compatibility for blobs.
Too large xattrs weren't written to disk before,
API for blobstore stays the same - only reporting ENOMEM
when it should.

Signed-off-by: default avatarTomasz Zawadzki <tomasz.zawadzki@intel.com>
Change-Id: I6f4af4d079e47f084e20d7a4969d9a78ec1f8610
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/460450


Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarBen Walker <benjamin.walker@intel.com>
Reviewed-by: default avatarMaciej Szwed <maciej.szwed@intel.com>
Reviewed-by: default avatarShuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: default avatarDarek Stojaczyk <dariusz.stojaczyk@intel.com>
parent 7ee58b90

lib/blob/blobstore.c

+8 −0
Original line number Diff line number Diff line
@@ -6061,6 +6061,7 @@ _spdk_blob_set_xattr(struct spdk_blob *blob, const char *name, const void *value 
{

	struct spdk_xattr_tailq *xattrs;

	struct spdk_xattr	*xattr;

	size_t			desc_size;



	_spdk_blob_verify_md_op(blob);
@@ -6068,6 +6069,13 @@ _spdk_blob_set_xattr(struct spdk_blob *blob, const char *name, const void *value 
		return -EPERM;

	}



	desc_size = sizeof(struct spdk_blob_md_descriptor_xattr) + strlen(name) + value_len;

	if (desc_size > SPDK_BS_MAX_DESC_SIZE) {

		SPDK_DEBUGLOG(SPDK_LOG_BLOB, "Xattr '%s' of size %ld does not fix into single page %ld\n", name,

			      desc_size, SPDK_BS_MAX_DESC_SIZE);

		return -ENOMEM;

	}



	if (internal) {

		xattrs = &blob->xattrs_internal;

		blob->invalid_flags |= SPDK_BLOB_INTERNAL_XATTR;

lib/blob/blobstore.h

+2 −0
Original line number Diff line number Diff line
@@ -321,6 +321,8 @@ struct spdk_blob_md_page { 
#define SPDK_BS_PAGE_SIZE 0x1000

SPDK_STATIC_ASSERT(SPDK_BS_PAGE_SIZE == sizeof(struct spdk_blob_md_page), "Invalid md page size");



#define SPDK_BS_MAX_DESC_SIZE sizeof(((struct spdk_blob_md_page*)0)->descriptors)



#define SPDK_BS_SUPER_BLOCK_SIG "SPDKBLOB"



struct spdk_bs_super_block {

test/unit/lib/blob/blob.c/blob_ut.c

+11 −0
Original line number Diff line number Diff line
@@ -4233,6 +4233,8 @@ blob_set_xattrs(void) 
	spdk_blob_id blobid;

	const void *value;

	size_t value_len;

	char *xattr;

	size_t xattr_length;

	int rc;



	dev = init_dev();
@@ -4289,6 +4291,15 @@ blob_set_xattrs(void) 
	rc = spdk_blob_get_xattr_value(blob, "foobar", &value, &value_len);

	CU_ASSERT(rc == -ENOENT);



	/* Try xattr exceeding maximum length of descriptor in single page */

	xattr_length = SPDK_BS_MAX_DESC_SIZE - sizeof(struct spdk_blob_md_descriptor_xattr) -

		       strlen("large_xattr") + 1;

	xattr = calloc(xattr_length, sizeof(char));

	SPDK_CU_ASSERT_FATAL(xattr != NULL);

	rc = spdk_blob_set_xattr(blob, "large_xattr", xattr, xattr_length);

	free(xattr);

	SPDK_CU_ASSERT_FATAL(rc == -ENOMEM);



	spdk_blob_close(blob, blob_op_complete, NULL);

	poll_threads();

	CU_ASSERT(g_bserrno == 0);