[PATCH 0/3] Handling extended headers of bnep control frames

[PATCH 0/3] Handling extended headers of bnep control frames

[Grzegorz Kolodziejczyk]

This patch set adds support for handling extended headers of bnep control
frames which is mandatory. Until now extended headers for control messages was
ignored.

Patch [1/3] adds setting of err variable if control message is not understood.
Patch [2/3] adds additional ioctl for get supported bnep features. This is
needed for user space to know if handling of sending success setup response
frame should be handled by kernel or by user space.
Patch [3/3] adds support for handling extended headers of bnep control frames.

Patch set tested with PTS bnep test cases. No regression, issues found during
testing.

Grzegorz Kolodziejczyk (3):
  Bluetooth: bnep: Return err value while sending cmd is not understood
  Bluetooth: bnep: Add support for get bnep features via ioctl
  Bluetooth: bnep: Add support to extended headers of control frames

 net/bluetooth/bnep/bnep.h |  4 ++++
 net/bluetooth/bnep/core.c | 57 +++++++++++++++++++++++++++++++++++++----------
 net/bluetooth/bnep/sock.c |  8 +++++++
 3 files changed, 57 insertions(+), 12 deletions(-)

-- 
2.1.0

[PATCH 1/3] Bluetooth: bnep: Return err value while sending cmd is not understood

[Grzegorz Kolodziejczyk]

Send command not understood response should be verified if it was
successfully sent, like all send responses.

Signed-off-by: Grzegorz Kolodziejczyk <grzegorz.kolodziejczyk@tieto.com>
---
 net/bluetooth/bnep/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/bluetooth/bnep/core.c b/net/bluetooth/bnep/core.c
index 05f57e4..c627817 100644
--- a/net/bluetooth/bnep/core.c
+++ b/net/bluetooth/bnep/core.c
@@ -239,7 +239,7 @@ static int bnep_rx_control(struct bnep_session *s, void *data, int len)
 			pkt[0] = BNEP_CONTROL;
 			pkt[1] = BNEP_CMD_NOT_UNDERSTOOD;
 			pkt[2] = cmd;
-			bnep_send(s, pkt, sizeof(pkt));
+			err = bnep_send(s, pkt, sizeof(pkt));
 		}
 		break;
 	}
-- 
2.1.0

[PATCH 2/3] Bluetooth: bnep: Add support for get bnep features via ioctl

[Grzegorz Kolodziejczyk]

This is needed if user space wants to know supported bnep features
by kernel, eg. if kernel supports sending response to bnep setup
control message.

Signed-off-by: Grzegorz Kolodziejczyk <grzegorz.kolodziejczyk@tieto.com>
---
 net/bluetooth/bnep/bnep.h | 4 ++++
 net/bluetooth/bnep/sock.c | 8 ++++++++
 2 files changed, 12 insertions(+)

diff --git a/net/bluetooth/bnep/bnep.h b/net/bluetooth/bnep/bnep.h
index 5a5b16f..a116e2e 100644
--- a/net/bluetooth/bnep/bnep.h
+++ b/net/bluetooth/bnep/bnep.h
@@ -80,6 +80,9 @@
 #define BNEP_TYPE_MASK	0x7f
 #define BNEP_EXT_HEADER	0x80
 
+/* Flags */
+#define BNEP_SETUP_RSP_FLAG	0x00000000
+
 struct bnep_setup_conn_req {
 	__u8 type;
 	__u8 ctrl;
@@ -111,6 +114,7 @@ struct bnep_ext_hdr {
 #define BNEPCONNDEL	_IOW('B', 201, int)
 #define BNEPGETCONNLIST	_IOR('B', 210, int)
 #define BNEPGETCONNINFO	_IOR('B', 211, int)
+#define BNEPGETSUPPFEAT	_IOR('B', 212, int)
 
 struct bnep_connadd_req {
 	int   sock;		/* Connected socket */
diff --git a/net/bluetooth/bnep/sock.c b/net/bluetooth/bnep/sock.c
index 5f05129..6b84baf 100644
--- a/net/bluetooth/bnep/sock.c
+++ b/net/bluetooth/bnep/sock.c
@@ -29,6 +29,8 @@
 
 #include "bnep.h"
 
+static const __u32 supp_feat = BIT(BNEP_SETUP_RSP_FLAG);
+
 static struct bt_sock_list bnep_sk_list = {
 	.lock = __RW_LOCK_UNLOCKED(bnep_sk_list.lock)
 };
@@ -120,6 +122,12 @@ static int bnep_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long
 
 		return err;
 
+	case BNEPGETSUPPFEAT:
+		if (copy_to_user(argp, &supp_feat, sizeof(supp_feat)))
+			return -EFAULT;
+
+		return 0;
+
 	default:
 		return -EINVAL;
 	}
-- 
2.1.0

[PATCH 3/3] Bluetooth: bnep: Add support to extended headers of control frames

[Grzegorz Kolodziejczyk]

Handling extended headers of control frames is required BNEP
functionality. Previously setup success response was handled in user
space and whole extended header was omitted. This patch allows to
handle setup success response and parsing extended headers in control
messages.

Signed-off-by: Grzegorz Kolodziejczyk <grzegorz.kolodziejczyk@tieto.com>
---
 net/bluetooth/bnep/core.c | 55 +++++++++++++++++++++++++++++++++++++----------
 1 file changed, 44 insertions(+), 11 deletions(-)

diff --git a/net/bluetooth/bnep/core.c b/net/bluetooth/bnep/core.c
index c627817..eb459da 100644
--- a/net/bluetooth/bnep/core.c
+++ b/net/bluetooth/bnep/core.c
@@ -231,7 +231,13 @@ static int bnep_rx_control(struct bnep_session *s, void *data, int len)
 		break;
 
 	case BNEP_SETUP_CONN_REQ:
-		err = bnep_send_rsp(s, BNEP_SETUP_CONN_RSP, BNEP_CONN_NOT_ALLOWED);
+		/* Successful response should be sent only once */
+		if (test_and_clear_bit(BNEP_SETUP_RSP_FLAG, &s->flags))
+			err = bnep_send_rsp(s, BNEP_SETUP_CONN_RSP,
+					    BNEP_SUCCESS);
+		else
+			err = bnep_send_rsp(s, BNEP_SETUP_CONN_RSP,
+					    BNEP_CONN_NOT_ALLOWED);
 		break;
 
 	default: {
@@ -292,29 +298,55 @@ static int bnep_rx_frame(struct bnep_session *s, struct sk_buff *skb)
 {
 	struct net_device *dev = s->dev;
 	struct sk_buff *nskb;
-	u8 type;
+	u8 type, ctrl_type;
 
 	dev->stats.rx_bytes += skb->len;
 
 	type = *(u8 *) skb->data;
 	skb_pull(skb, 1);
+	ctrl_type = *(u8 *)skb->data;
 
 	if ((type & BNEP_TYPE_MASK) >= sizeof(__bnep_rx_hlen))
 		goto badframe;
 
 	if ((type & BNEP_TYPE_MASK) == BNEP_CONTROL) {
-		bnep_rx_control(s, skb->data, skb->len);
-		kfree_skb(skb);
-		return 0;
-	}
+		if (bnep_rx_control(s, skb->data, skb->len) < 0) {
+			dev->stats.tx_errors++;
+			kfree_skb(skb);
+			return 0;
+		}
 
-	skb_reset_mac_header(skb);
+		if (!(type & BNEP_EXT_HEADER)) {
+			kfree_skb(skb);
+			return 0;
+		}
 
-	/* Verify and pull out header */
-	if (!skb_pull(skb, __bnep_rx_hlen[type & BNEP_TYPE_MASK]))
-		goto badframe;
+		/* Verify and pull ctrl message since it's already processed */
+		switch (ctrl_type) {
+		case BNEP_SETUP_CONN_REQ:
+			/* Pull: ctrl type (1 b), len (1 b), data (len bytes) */
+			if (!skb_pull(skb, 2 + *(u8 *)(skb->data + 1) * 2))
+				goto badframe;
+			break;
+		case BNEP_FILTER_MULTI_ADDR_SET:
+		case BNEP_FILTER_NET_TYPE_SET:
+			/* Pull: ctrl type (1 b), len (2 b), data (len bytes) */
+			if (!skb_pull(skb, 3 + *(u16 *)(skb->data + 1) * 2))
+				goto badframe;
+			break;
+		default:
+			kfree_skb(skb);
+			return 0;
+		}
+	} else {
+		skb_reset_mac_header(skb);
 
-	s->eh.h_proto = get_unaligned((__be16 *) (skb->data - 2));
+		/* Verify and pull out header */
+		if (!skb_pull(skb, __bnep_rx_hlen[type & BNEP_TYPE_MASK]))
+			goto badframe;
+
+		s->eh.h_proto = get_unaligned((__be16 *) (skb->data - 2));
+	}
 
 	if (type & BNEP_EXT_HEADER) {
 		if (bnep_rx_extension(s, skb) < 0)
@@ -566,6 +598,7 @@ int bnep_add_connection(struct bnep_connadd_req *req, struct socket *sock)
 	s->sock  = sock;
 	s->role  = req->role;
 	s->state = BT_CONNECTED;
+	s->flags = req->flags;
 
 	s->msg.msg_flags = MSG_NOSIGNAL;
 
-- 
2.1.0

Re: [PATCH 2/3] Bluetooth: bnep: Add support for get bnep features via ioctl

[Marcel Holtmann]

Hi Grzegorz,

> This is needed if user space wants to know supported bnep features
> by kernel, eg. if kernel supports sending response to bnep setup
> control message.
> 
> Signed-off-by: Grzegorz Kolodziejczyk <grzegorz.kolodziejczyk@tieto.com>
> ---
> net/bluetooth/bnep/bnep.h | 4 ++++
> net/bluetooth/bnep/sock.c | 8 ++++++++
> 2 files changed, 12 insertions(+)
> 
> diff --git a/net/bluetooth/bnep/bnep.h b/net/bluetooth/bnep/bnep.h
> index 5a5b16f..a116e2e 100644
> --- a/net/bluetooth/bnep/bnep.h
> +++ b/net/bluetooth/bnep/bnep.h
> @@ -80,6 +80,9 @@
> #define BNEP_TYPE_MASK	0x7f
> #define BNEP_EXT_HEADER	0x80
> 
> +/* Flags */
> +#define BNEP_SETUP_RSP_FLAG	0x00000000
> +
> struct bnep_setup_conn_req {
> 	__u8 type;
> 	__u8 ctrl;
> @@ -111,6 +114,7 @@ struct bnep_ext_hdr {
> #define BNEPCONNDEL	_IOW('B', 201, int)
> #define BNEPGETCONNLIST	_IOR('B', 210, int)
> #define BNEPGETCONNINFO	_IOR('B', 211, int)
> +#define BNEPGETSUPPFEAT	_IOR('B', 212, int)

as mentioned on IRC, you forgot compat_ioctl.h

> struct bnep_connadd_req {
> 	int   sock;		/* Connected socket */
> diff --git a/net/bluetooth/bnep/sock.c b/net/bluetooth/bnep/sock.c
> index 5f05129..6b84baf 100644
> --- a/net/bluetooth/bnep/sock.c
> +++ b/net/bluetooth/bnep/sock.c
> @@ -29,6 +29,8 @@
> 
> #include "bnep.h"
> 
> +static const __u32 supp_feat = BIT(BNEP_SETUP_RSP_FLAG);
> +

I am confused with the naming here. Are we exposing current state or supported features or what?

Regards

Marcel