Bluetooth: ISO: unlock on error path in iso_sock_setsockopt()

Call release_sock(sk); before returning on this error path.

---
 net/bluetooth/iso.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

index ff09c353e64e..19d003727b50 100644
@@ -1177,8 +1177,10 @@ static int iso_sock_setsockopt(struct socket *sock, int level, int optname,
 		}
 
 		len = min_t(unsigned int, sizeof(qos), optlen);
-		if (len != sizeof(qos))
-			return -EINVAL;
+		if (len != sizeof(qos)) {
+			err = -EINVAL;
+			break;
+		}
 
 		memset(&qos, 0, sizeof(qos));
 

2.35.1

The "qos" struct has holes after the in and out struct members. Zero out those holes to prevent leaking stack information.

The C standard rules for when struct holes are zeroed out are slightly weird. The existing assignments might initialize everything, but GCC is allowed to (and does sometimes) leave the struct holes uninitialized. However, when you have a struct initializer that doesn't initialize every member then the holes must be zeroed.

---
 net/bluetooth/iso.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 19d003727b50..c982087d3b52 100644
@@ -1235,7 +1235,7 @@ static int iso_sock_getsockopt(struct socket *sock, int level, int optname,
 {
 	struct sock *sk = sock->sk;
 	int len, err = 0;
-	struct bt_iso_qos qos;
+	struct bt_iso_qos qos = {}; /* zero out struct holes */
 	u8 base_len;
 	u8 *base;
 

2.35.1

Hi Dan,

Interesting, did you get a report from static analyzer or something? The variable gets assigned in the code below which has the exact same size thus I don't see how it would leave anything uninitialized:

        if (sk->sk_state == BT_CONNECTED || sk->sk_state == BT_CONNECT2)
            qos = iso_pi(sk)->conn->hcon->iso_qos;
        else
            qos = iso_pi(sk)->qos;

Well perhaps it would have been better to use a pointer though so we don't have to copy anything:

index ff09c353e64e..0e4ec46ef273 100644
@@ -1233,7 +1233,7 @@ static int iso_sock_getsockopt(struct socket

*sock, int level, int optname,

 {
        struct sock *sk = sock->sk;
        int len, err = 0;
-       struct bt_iso_qos qos;
+       struct bt_iso_qos *qos;
        u8 base_len;
        u8 *base;

@@ -1259,12 +1259,12 @@ static int iso_sock_getsockopt(struct socket *sock, int level, int optname,

        case BT_ISO_QOS:
                if (sk->sk_state == BT_CONNECTED || sk->sk_state == BT_CONNECT2)

- qos = iso_pi(sk)->conn->hcon->iso_qos;
+ qos = &iso_pi(sk)->conn->hcon->iso_qos;
else
- qos = iso_pi(sk)->qos;
+ qos = &iso_pi(sk)->qos;

                len = min_t(unsigned int, len, sizeof(qos));
-               if (copy_to_user(optval, (char *)&qos, len))
+               if (copy_to_user(optval, (char *)qos, len))
                        err = -EFAULT;

                break;

Luiz Augusto von Dentz

Yeah. It's a Smatch check. Unfortunately, it still complains after my patch... Which is frustrating because I thought I had fixed that.

It's the struct holes after ->in and ->out which are the issue. When you have an assignment like that, the compiler is allowed to do it as a series of assignments:

	foo = bar;

becomes:

	foo.a = bar.a;
	foo.b = bar.b;
	foo.c = bar.c;

That works, and it's faster too. Do you want to send that and give me a Reported-by tag? Otherwise I can.

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list. This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=663427

---Test result---

Test Summary:

CheckPatch                    PASS      3.49 seconds
GitLint                       PASS      1.67 seconds
SubjectPrefix                 PASS      1.21 seconds
BuildKernel                   PASS      35.53 seconds
BuildKernel32                 PASS      31.02 seconds
Incremental Build with patchesPASS      48.90 seconds
TestRunner: Setup             PASS      509.47 seconds
TestRunner: l2cap-tester      PASS      17.95 seconds
TestRunner: bnep-tester       PASS      6.84 seconds
TestRunner: mgmt-tester       PASS      106.23 seconds
TestRunner: rfcomm-tester     PASS      10.13 seconds
TestRunner: sco-tester        PASS      9.97 seconds
TestRunner: smp-tester        PASS      9.88 seconds
TestRunner: userchan-tester   PASS      6.87 seconds

---
Regards,
Linux Bluetooth