Received-SPF: pass (google.com: domain of linux-bluetooth+bounces-576-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
From: Dmitry Antipov <dmantipov@yandex.ru>
To: Marcel Holtmann <marcel@holtmann.org>
Cc: Johan Hedberg <johan.hedberg@gmail.com>,
	linux-bluetooth@vger.kernel.org,
	Dmitry Antipov <dmantipov@yandex.ru>
Subject: [PATCH] bluetooth: bnep: fix fortify warning
Date: Wed, 13 Dec 2023 18:42:57 +0300
Message-ID: <20231213154303.159184-1-dmantipov@yandex.ru>
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit

When compiling with gcc version 14.0.0 20231206 (experimental)
and CONFIG_FORTIFY_SOURCE=y, I've noticed the following warning:

In function 'fortify_memcpy_chk',
    inlined from '__skb_put_data' at ./include/linux/skbuff.h:2599:2,
    inlined from 'bnep_rx_frame.isra' at net/bluetooth/bnep/core.c:388:3:
./include/linux/fortify-string.h:588:25: warning: call to '__read_overflow2_field'
declared with attribute warning: detected read beyond size of field (2nd parameter);
maybe use struct_group()? [-Wattribute-warning]
  588 |                         __read_overflow2_field(q_size_field, size);
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

There is a 'memcpy()' call underneath '__skb_put_data()', and the former
is interpreted as an attempt to copy ETH_ALEN + 2 (which is 8) bytes from
the 6-byte 'h_source' field of 'struct ethhdr', which causes an overread
warning. The convenient way to avoid it is to use 'struct_group()', i.e.:

struct ethhdr {
	unsigned char	h_dest[ETH_ALEN];
	struct_group(xxx,
	        unsigned char	h_source[ETH_ALEN];
	        __be16		h_proto;
        );
} __attribute__((packed));

But since 'struct ethhdr' is a fundamental type and most likely it would be
a bad idea to mess it up that way just for the sake of a few bluetooth bits,
I would suggest an ad-hoc quirk instead.

Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
 net/bluetooth/bnep/core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/bluetooth/bnep/core.c b/net/bluetooth/bnep/core.c
index 5a6a49885ab6..8edceb4d2a4f 100644
--- a/net/bluetooth/bnep/core.c
+++ b/net/bluetooth/bnep/core.c
@@ -385,7 +385,8 @@ static int bnep_rx_frame(struct bnep_session *s, struct sk_buff *skb)
 
 	case BNEP_COMPRESSED_DST_ONLY:
 		__skb_put_data(nskb, skb_mac_header(skb), ETH_ALEN);
-		__skb_put_data(nskb, s->eh.h_source, ETH_ALEN + 2);
+		__skb_put_data(nskb, (unsigned char *)&s->eh + ETH_ALEN,
+			       ETH_ALEN + 2);
 		break;
 
 	case BNEP_GENERAL:
-- 
2.43.0