nbd: Don't take address of fields in packed structs
Taking the address of a field in a packed struct is a bad idea, because
it might not be actually aligned enough for that pointer type (and
thus cause a crash on dereference on some host architectures). Newer
versions of clang warn about this. Avoid the bug by not using the
"modify in place" byte swapping functions.
This patch was produced with the following spatch script:
@@
expression E;
@@
-be16_to_cpus(&E);
+E = be16_to_cpu(E);
@@
expression E;
@@
-be32_to_cpus(&E);
+E = be32_to_cpu(E);
@@
expression E;
@@
-be64_to_cpus(&E);
+E = be64_to_cpu(E);
@@
expression E;
@@
-cpu_to_be16s(&E);
+E = cpu_to_be16(E);
@@
expression E;
@@
-cpu_to_be32s(&E);
+E = cpu_to_be32(E);
@@
expression E;
@@
-cpu_to_be64s(&E);
+E = cpu_to_be64(E);
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <20180927164200.15097-1-peter.maydell@linaro.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
[eblake: rebase, and squash in missed changes]
Signed-off-by: Eric Blake <eblake@redhat.com>
diff --git a/nbd/client.c b/nbd/client.c
index 40b74d9..b4d457a 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -117,10 +117,10 @@
nbd_send_opt_abort(ioc);
return -1;
}
- be64_to_cpus(&reply->magic);
- be32_to_cpus(&reply->option);
- be32_to_cpus(&reply->type);
- be32_to_cpus(&reply->length);
+ reply->magic = be64_to_cpu(reply->magic);
+ reply->option = be32_to_cpu(reply->option);
+ reply->type = be32_to_cpu(reply->type);
+ reply->length = be32_to_cpu(reply->length);
trace_nbd_receive_option_reply(reply->option, nbd_opt_lookup(reply->option),
reply->type, nbd_rep_lookup(reply->type),
@@ -396,7 +396,7 @@
return -1;
}
len -= sizeof(type);
- be16_to_cpus(&type);
+ type = be16_to_cpu(type);
switch (type) {
case NBD_INFO_EXPORT:
if (len != sizeof(info->size) + sizeof(info->flags)) {
@@ -410,13 +410,13 @@
nbd_send_opt_abort(ioc);
return -1;
}
- be64_to_cpus(&info->size);
+ info->size = be64_to_cpu(info->size);
if (nbd_read(ioc, &info->flags, sizeof(info->flags), errp) < 0) {
error_prepend(errp, "failed to read info flags: ");
nbd_send_opt_abort(ioc);
return -1;
}
- be16_to_cpus(&info->flags);
+ info->flags = be16_to_cpu(info->flags);
trace_nbd_receive_negotiate_size_flags(info->size, info->flags);
break;
@@ -433,7 +433,7 @@
nbd_send_opt_abort(ioc);
return -1;
}
- be32_to_cpus(&info->min_block);
+ info->min_block = be32_to_cpu(info->min_block);
if (!is_power_of_2(info->min_block)) {
error_setg(errp, "server minimum block size %" PRIu32
" is not a power of two", info->min_block);
@@ -447,7 +447,7 @@
nbd_send_opt_abort(ioc);
return -1;
}
- be32_to_cpus(&info->opt_block);
+ info->opt_block = be32_to_cpu(info->opt_block);
if (!is_power_of_2(info->opt_block) ||
info->opt_block < info->min_block) {
error_setg(errp, "server preferred block size %" PRIu32
@@ -461,7 +461,7 @@
nbd_send_opt_abort(ioc);
return -1;
}
- be32_to_cpus(&info->max_block);
+ info->max_block = be32_to_cpu(info->max_block);
if (info->max_block < info->min_block) {
error_setg(errp, "server maximum block size %" PRIu32
" is not valid", info->max_block);
@@ -668,7 +668,7 @@
if (nbd_read(ioc, &received_id, sizeof(received_id), errp) < 0) {
return -1;
}
- be32_to_cpus(&received_id);
+ received_id = be32_to_cpu(received_id);
reply.length -= sizeof(received_id);
name = g_malloc(reply.length + 1);
@@ -872,13 +872,13 @@
error_prepend(errp, "Failed to read export length: ");
goto fail;
}
- be64_to_cpus(&info->size);
+ info->size = be64_to_cpu(info->size);
if (nbd_read(ioc, &info->flags, sizeof(info->flags), errp) < 0) {
error_prepend(errp, "Failed to read export flags: ");
goto fail;
}
- be16_to_cpus(&info->flags);
+ info->flags = be16_to_cpu(info->flags);
} else if (magic == NBD_CLIENT_MAGIC) {
uint32_t oldflags;
@@ -895,13 +895,13 @@
error_prepend(errp, "Failed to read export length: ");
goto fail;
}
- be64_to_cpus(&info->size);
+ info->size = be64_to_cpu(info->size);
if (nbd_read(ioc, &oldflags, sizeof(oldflags), errp) < 0) {
error_prepend(errp, "Failed to read export flags: ");
goto fail;
}
- be32_to_cpus(&oldflags);
+ oldflags = be32_to_cpu(oldflags);
if (oldflags & ~0xffff) {
error_setg(errp, "Unexpected export flags %0x" PRIx32, oldflags);
goto fail;
@@ -1080,8 +1080,8 @@
return ret;
}
- be32_to_cpus(&reply->error);
- be64_to_cpus(&reply->handle);
+ reply->error = be32_to_cpu(reply->error);
+ reply->handle = be64_to_cpu(reply->handle);
return 0;
}
@@ -1105,10 +1105,10 @@
return ret;
}
- be16_to_cpus(&chunk->flags);
- be16_to_cpus(&chunk->type);
- be64_to_cpus(&chunk->handle);
- be32_to_cpus(&chunk->length);
+ chunk->flags = be16_to_cpu(chunk->flags);
+ chunk->type = be16_to_cpu(chunk->type);
+ chunk->handle = be64_to_cpu(chunk->handle);
+ chunk->length = be32_to_cpu(chunk->length);
return 0;
}
@@ -1128,7 +1128,7 @@
return ret;
}
- be32_to_cpus(&reply->magic);
+ reply->magic = be32_to_cpu(reply->magic);
switch (reply->magic) {
case NBD_SIMPLE_REPLY_MAGIC:
diff --git a/nbd/server.c b/nbd/server.c
index c3dd402..98d0fa2 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -333,7 +333,7 @@
if (ret <= 0) {
return ret;
}
- cpu_to_be32s(&len);
+ len = cpu_to_be32(len);
if (len > NBD_MAX_NAME_SIZE) {
return nbd_opt_invalid(client, errp,
@@ -486,7 +486,7 @@
if (rc < 0) {
return rc;
}
- cpu_to_be16s(&info);
+ info = cpu_to_be16(info);
if (nbd_write(client->ioc, &info, sizeof(info), errp) < 0) {
return -EIO;
}
@@ -551,14 +551,14 @@
if (rc <= 0) {
return rc;
}
- be16_to_cpus(&requests);
+ requests = be16_to_cpu(requests);
trace_nbd_negotiate_handle_info_requests(requests);
while (requests--) {
rc = nbd_opt_read(client, &request, sizeof(request), errp);
if (rc <= 0) {
return rc;
}
- be16_to_cpus(&request);
+ request = be16_to_cpu(request);
trace_nbd_negotiate_handle_info_request(request,
nbd_info_lookup(request));
/* We care about NBD_INFO_NAME and NBD_INFO_BLOCK_SIZE;
@@ -618,9 +618,9 @@
/* maximum - At most 32M, but smaller as appropriate. */
sizes[2] = MIN(blk_get_max_transfer(exp->blk), NBD_MAX_BUFFER_SIZE);
trace_nbd_negotiate_handle_info_block_size(sizes[0], sizes[1], sizes[2]);
- cpu_to_be32s(&sizes[0]);
- cpu_to_be32s(&sizes[1]);
- cpu_to_be32s(&sizes[2]);
+ sizes[0] = cpu_to_be32(sizes[0]);
+ sizes[1] = cpu_to_be32(sizes[1]);
+ sizes[2] = cpu_to_be32(sizes[2]);
rc = nbd_negotiate_send_info(client, NBD_INFO_BLOCK_SIZE,
sizeof(sizes), sizes, errp);
if (rc < 0) {
@@ -904,7 +904,7 @@
if (ret <= 0) {
return ret;
}
- cpu_to_be32s(&len);
+ len = cpu_to_be32(len);
if (len < ns_len) {
trace_nbd_negotiate_meta_query_skip("length too short");
@@ -971,7 +971,7 @@
if (ret <= 0) {
return ret;
}
- cpu_to_be32s(&nb_queries);
+ nb_queries = cpu_to_be32(nb_queries);
trace_nbd_negotiate_meta_context(nbd_opt_lookup(client->opt),
export_name, nb_queries);
@@ -1049,7 +1049,7 @@
error_prepend(errp, "read failed: ");
return -EIO;
}
- be32_to_cpus(&flags);
+ flags = be32_to_cpu(flags);
trace_nbd_negotiate_options_flags(flags);
if (flags & NBD_FLAG_C_FIXED_NEWSTYLE) {
fixedNewstyle = true;
@@ -1900,8 +1900,8 @@
extents_end = extent + 1;
for (extent = extents; extent < extents_end; extent++) {
- cpu_to_be32s(&extent->flags);
- cpu_to_be32s(&extent->length);
+ extent->flags = cpu_to_be32(extent->flags);
+ extent->length = cpu_to_be32(extent->length);
}
*bytes -= remaining_bytes;
