All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vadim Troshchinskiy <vtroshchinskiy@qindel.com>
To: "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>
Cc: Valentina Manea <valentina.manea.m@gmail.com>,
	Shuah Khan <shuah@kernel.org>
Subject: [PATCH] usbip: Remove unaligned pointer usage from usbip tools
Date: Tue, 10 Dec 2019 16:50:45 +0100	[thread overview]
Message-ID: <5176009.64u6Zm7RkX@gverdu.qindel.com> (raw)

The usbip tools use packed structs for network communication. Taking the
address of a packed member of a struct can crash the program with SIGBUS
on architectures with strict alignment requirements.

Also, recent versions of GCC detect this situation and emit a warning that
is fatal due to -Werror being used.

error: taking address of packed member of ‘struct
usbip_usb_device’ may result in an unaligned pointer value [-Werror=address-
of-packed-member]

Fix this by copying the data to an aligned location and operating there.

Signed-off-by: Vadim Troshchinskiy <vtroshchinskiy@qindel.com>
---
 tools/usb/usbip/src/usbip_network.c | 30 +++++++++++++++--------------
 tools/usb/usbip/src/usbip_network.h | 12 ++++++------
 2 files changed, 22 insertions(+), 20 deletions(-)

diff --git a/tools/usb/usbip/src/usbip_network.c b/tools/usb/usbip/src/usbip_network.c
index d595d72693fb..1c0038ee0abd 100644
--- a/tools/usb/usbip/src/usbip_network.c
+++ b/tools/usb/usbip/src/usbip_network.c
@@ -50,39 +50,41 @@ void usbip_setup_port_number(char *arg)
 	info("using port %d (\"%s\")", usbip_port, usbip_port_string);
 }
 
-void usbip_net_pack_uint32_t(int pack, uint32_t *num)
+void usbip_net_pack_uint32_t(int pack, uint8_t *num)
 {
 	uint32_t i;
+	memcpy(&i, num, sizeof(i));
 
 	if (pack)
-		i = htonl(*num);
+		i = htonl(i);
 	else
-		i = ntohl(*num);
+		i = ntohl(i);
 
-	*num = i;
+	memcpy(num, &i, sizeof(i));
 }
 
-void usbip_net_pack_uint16_t(int pack, uint16_t *num)
+void usbip_net_pack_uint16_t(int pack, uint8_t *num)
 {
 	uint16_t i;
+	memcpy(&i, num, sizeof(i));
 
 	if (pack)
-		i = htons(*num);
+		i = htons(i);
 	else
-		i = ntohs(*num);
+		i = ntohs(i);
 
-	*num = i;
+	memcpy(num, &i, sizeof(i));
 }
 
 void usbip_net_pack_usb_device(int pack, struct usbip_usb_device *udev)
 {
-	usbip_net_pack_uint32_t(pack, &udev->busnum);
-	usbip_net_pack_uint32_t(pack, &udev->devnum);
-	usbip_net_pack_uint32_t(pack, &udev->speed);
+	usbip_net_pack_uint32_t(pack, (uint8_t*)&udev->busnum);
+	usbip_net_pack_uint32_t(pack, (uint8_t*)&udev->devnum);
+	usbip_net_pack_uint32_t(pack, (uint8_t*)&udev->speed);
 
-	usbip_net_pack_uint16_t(pack, &udev->idVendor);
-	usbip_net_pack_uint16_t(pack, &udev->idProduct);
-	usbip_net_pack_uint16_t(pack, &udev->bcdDevice);
+	usbip_net_pack_uint16_t(pack, (uint8_t*)&udev->idVendor);
+	usbip_net_pack_uint16_t(pack, (uint8_t*)&udev->idProduct);
+	usbip_net_pack_uint16_t(pack, (uint8_t*)&udev->bcdDevice);
 }
 
 void usbip_net_pack_usb_interface(int pack __attribute__((unused)),
diff --git a/tools/usb/usbip/src/usbip_network.h b/tools/usb/usbip/src/usbip_network.h
index 555215eae43e..821dd65877cc 100644
--- a/tools/usb/usbip/src/usbip_network.h
+++ b/tools/usb/usbip/src/usbip_network.h
@@ -33,9 +33,9 @@ struct op_common {
 } __attribute__((packed));
 
 #define PACK_OP_COMMON(pack, op_common)  do {\
-	usbip_net_pack_uint16_t(pack, &(op_common)->version);\
-	usbip_net_pack_uint16_t(pack, &(op_common)->code);\
-	usbip_net_pack_uint32_t(pack, &(op_common)->status);\
+	usbip_net_pack_uint16_t(pack, (uint8_t*)&(op_common)->version);\
+	usbip_net_pack_uint16_t(pack, (uint8_t*)&(op_common)->code);\
+	usbip_net_pack_uint32_t(pack, (uint8_t*)&(op_common)->status);\
 } while (0)
 
 /* ---------------------------------------------------------------------- */
@@ -163,11 +163,11 @@ struct op_devlist_reply_extra {
 } while (0)
 
 #define PACK_OP_DEVLIST_REPLY(pack, reply)  do {\
-	usbip_net_pack_uint32_t(pack, &(reply)->ndev);\
+	usbip_net_pack_uint32_t(pack, (uint8_t*)&(reply)->ndev);\
 } while (0)
 
-void usbip_net_pack_uint32_t(int pack, uint32_t *num);
-void usbip_net_pack_uint16_t(int pack, uint16_t *num);
+void usbip_net_pack_uint32_t(int pack, uint8_t *num);
+void usbip_net_pack_uint16_t(int pack, uint8_t *num);
 void usbip_net_pack_usb_device(int pack, struct usbip_usb_device *udev);
 void usbip_net_pack_usb_interface(int pack, struct usbip_usb_interface *uinf);
 
-- 
2.21.0





             reply	other threads:[~2019-12-10 15:50 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-10 15:50 Vadim Troshchinskiy [this message]
2020-01-02  0:01 ` [PATCH] usbip: Remove unaligned pointer usage from usbip tools shuah
2020-01-15  8:52   ` Vadim Troshchinskiy
2020-01-17 16:09     ` shuah

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5176009.64u6Zm7RkX@gverdu.qindel.com \
    --to=vtroshchinskiy@qindel.com \
    --cc=linux-usb@vger.kernel.org \
    --cc=shuah@kernel.org \
    --cc=valentina.manea.m@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.