Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 29 Jul 2022 11:00:47 +0300
From:   Dan Carpenter <dan.carpenter@oracle.com>
To:     luiz.von.dentz@intel.com,
        Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
Cc:     linux-bluetooth@vger.kernel.org
Subject: [bug report] Bluetooth: ISO: Add broadcast support
Message-ID: <YuOTr1AAJq3dMmyY@kili>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?tkdtsWfGoNivYtE7H1YR4vsBc9+05XazSMx9edO6Pzuwp4vhtz5ybq28iZ99?=
 =?us-ascii?Q?Mbs+y0DN5C2nrCX/rpiKcD+XDFwPxyd2LKYi3ySVmdG9iWRXAw1jtwz0nkYr?=
 =?us-ascii?Q?ZuYH1EMfqJGPX72P47UPrrxkKOgdO8KhDHgJMh/1QgQ5KTnm2+dFJP5LmUgY?=
 =?us-ascii?Q?O7vFhS86UGIVNVDFIS3ZkQwrG+Del44HP9zqWA/hF6uP71cJ/eeaEMrLDkqs?=
 =?us-ascii?Q?TwfvUYZUdDCWQWJ9yzhNHL5YQi/4QZeKwp0z4O6tCxXXz4t6cvzXAa8Y3qJ1?=
 =?us-ascii?Q?/Zc5+WQjjyBQFeKI682S3KHQmCzbWKqBelgvdAIif0dHB4IH72SBrYsjzQZw?=
 =?us-ascii?Q?Q3ePQ2K5WZJBReK2bSca6M+RdoE04i2VhqJV9o88/vHrO+mfJ31g8cflNdH6?=
 =?us-ascii?Q?dgUNEc4qf90JMwp5anoPI0tbHGoKV/M7tF9ATGA+QI+zXNwNFo/GRzB9h+E3?=
 =?us-ascii?Q?OIhIGyV2tBahhxjoaol49sDrauoY19iIOH9ka/OMSZ9/rZsWplNo369c5sKX?=
 =?us-ascii?Q?XTHzj294gkCnijjTACaIKBWpGlaaUyi6Z//cvWAHYX989ysPt5epYJ/wmXEd?=
 =?us-ascii?Q?mtNHihxuX3DSPjInKlKCj8Ri4j7Gmfo3oA3iutK+KvtuHlhqR/p3ntTKDWP/?=
 =?us-ascii?Q?RGEasmbbbOztbAAK+kmnYpNLvOkJK7lyZ2ChgiyM/4dKpjlXRMMBbrFWIcwA?=
 =?us-ascii?Q?fg9YRnlbv6OtqNrl2MLEaSIhLTqUVL5Ik0FClqWExRwW60Cs4e3LEWGdH7mU?=
 =?us-ascii?Q?LELauY1FIowClhmM8gDGtBCv4FGKYNki1S/dl3c2bmG0ms9suRSIHhsx/DKi?=
 =?us-ascii?Q?2vZcG1jjeDFUmwLcH8Q1Lr3eidn+CkqcDxNxO2EGSWbS+zSXZGwejMeDckPS?=
 =?us-ascii?Q?ycap0JniXXtSpNb98mmBUwwAxY95yYZmorSnwutWOvq31T55kAfJYbLuaAc5?=
 =?us-ascii?Q?YtmASPmW+ZVEB1ICiOV73l4OOTqqcTEs7QDE1uDc+sA28MteWDYOj+EwLZHC?=
 =?us-ascii?Q?PdZwIY7v4COyoGJ2NRWUCPAyWEEJXvzMJT769mBDTImqey/2i0wmrubrpPPr?=
 =?us-ascii?Q?eBUsG0Yn+eACSIF9PaIrvXmZQv9XbX6JdqLiIR9xwXa7owc3BGyoh3grmwEr?=
 =?us-ascii?Q?umAqf50GDJUt6V2gIUZ8jrxQv5IyHO3wgNGovuwE8ZcxIw/Ut+CJZgMsHpnf?=
 =?us-ascii?Q?pyC6b+VdGrJ8BkYWg/ldBT1Apk+XuWkEgl2Y1dCfWrWxJgQzuunsP9esoojJ?=
 =?us-ascii?Q?QN3UeJMKae+VnRZMKV1VZ/GfzOWrcCEP2u8Hu2B9S+thL7tv22M/jvzEgScO?=
 =?us-ascii?Q?mbUNztoQvPaTc7CqIM1Htk7g1ANroIzRPW0NDtdpkPZXSSoPfE+Ad28wK5Ov?=
 =?us-ascii?Q?TxUgLa/v+yPROm8ri3CRuZ2JpAMnxXHxIfxehPyHRFmzTiqUG6xyzjJEr6IM?=
 =?us-ascii?Q?w0aKgu1JpDtmDYkYeiSXvpv+pZyjGhl/F7ba0aCDUoZ4ClDFTWclyFGubvKl?=
 =?us-ascii?Q?1836ic6o5tTBV58MEBHkAaWFs297086chgd/1VNvCMz5hhiprFDdkKlf/7Cx?=
 =?us-ascii?Q?8bJhCUu2hOycEVtk+796vJa4wHtyk11ojoDh9jdY8h5R1n0MwcU4cumTtmBF?=
 =?us-ascii?Q?pw=3D=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 9f68c8e3-548a-4c2f-e2b6-08da71387774
X-MS-Exchange-CrossTenant-AuthSource: MWHPR1001MB2365.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 29 Jul 2022 08:00:56.4477
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: qveuYKvayfaBvzhwPHmFNcM5IDjx8LmQ/cz7knfOjeK7KBK47y9Xz4O0Lv6GPCe25Ixrk5hf8ploiMZaKdASO4dIM+L2OwlYHVagYmX6D4g=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR10MB4680
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.205,Aquarius:18.0.883,Hydra:6.0.517,FMLib:17.11.122.1
 definitions=2022-07-28_06,2022-07-28_02,2022-06-22_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 phishscore=0 bulkscore=0 malwarescore=0
 adultscore=0 mlxscore=0 mlxlogscore=999 suspectscore=0 spamscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2206140000
 definitions=main-2207290032
X-Proofpoint-ORIG-GUID: fa32fymB_rQf5vhAQ0v60oGLKdwXK8WO
X-Proofpoint-GUID: fa32fymB_rQf5vhAQ0v60oGLKdwXK8WO
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-bluetooth.vger.kernel.org>
X-Mailing-List: linux-bluetooth@vger.kernel.org

Hi Luiz,

Harshit Mogalapalli brought this memory corruption issue to me.  Can you
take a look?  I don't know how to fix it.

The patch f764a6c2c1e4: "Bluetooth: ISO: Add broadcast support" from
Mar 9, 2022, leads to the following Smatch static checker warning:

	net/bluetooth/iso.c:1282 iso_sock_getsockopt()
	error: copy_to_user() 'base' too small (252 vs 254)

That warning is because Smatch gets confused but in reviewing the code,
it turns out that Smatch is correct (like a stopped clock which is
correct by accident).  The actual bug happens earlier in
eir_append_service_data().

Step 1:	iso_sock_setsockopt() sets ->base_len to 0-252

net/bluetooth/iso.c
  1208                  if (optlen > sizeof(iso_pi(sk)->base)) {
                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  1209                          err = -EOVERFLOW;
  1210                          break;
  1211                  }
  1212  
  1213                  len = min_t(unsigned int, sizeof(iso_pi(sk)->base), optlen);
  1214  
  1215                  if (copy_from_sockptr(iso_pi(sk)->base, optval, len)) {
  1216                          err = -EFAULT;
  1217                          break;
  1218                  }
  1219  
  1220                  iso_pi(sk)->base_len = len;
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^

Step 2: iso_connect_bis() passes ->base_len to hci_connect_bis()

net/bluetooth/iso.c
   235  static int iso_connect_bis(struct sock *sk)
   236  {
   237          struct iso_conn *conn;
   238          struct hci_conn *hcon;
   239          struct hci_dev  *hdev;
   240          int err;
   241  
   242          BT_DBG("%pMR", &iso_pi(sk)->src);
   243  
   244          hdev = hci_get_route(&iso_pi(sk)->dst, &iso_pi(sk)->src,
   245                               iso_pi(sk)->src_type);
   246          if (!hdev)
   247                  return -EHOSTUNREACH;
   248  
   249          hci_dev_lock(hdev);
   250  
   251          if (!bis_capable(hdev)) {
   252                  err = -EOPNOTSUPP;
   253                  goto done;
   254          }
   255  
   256          /* Fail if out PHYs are marked as disabled */
   257          if (!iso_pi(sk)->qos.out.phy) {
   258                  err = -EINVAL;
   259                  goto done;
   260          }
   261  
   262          hcon = hci_connect_bis(hdev, &iso_pi(sk)->dst, iso_pi(sk)->dst_type,
   263                                 &iso_pi(sk)->qos, iso_pi(sk)->base_len,
                                                         ^^^^^^^^^^^^^^^^^^^^^
   264                                 iso_pi(sk)->base);
   265          if (IS_ERR(hcon)) {
   266                  err = PTR_ERR(hcon);

Step 3:  hci_connect_bis() passes base_len to eir_append_service_data().
The buffer here is ->le_per_adv_data which is also size 252 bytes.

net/bluetooth/hci_conn.c
  2058          /* Add Basic Announcement into Peridic Adv Data if BASE is set */
  2059          if (base_len && base) {
  2060                  base_len = eir_append_service_data(conn->le_per_adv_data, 0,
                                                           ^^^^^^^^^^^^^^^^^^^^^
  2061                                                     0x1851, base, base_len);
                                                                         ^^^^^^^^
  2062                  conn->le_per_adv_data_len = base_len;
  2063          }

Step 4: memory corruption in eir_append_service_data()

net/bluetooth/eir.c
    69  u8 eir_append_service_data(u8 *eir, u16 eir_len, u16 uuid, u8 *data,
    70                             u8 data_len)
    71  {
    72          eir[eir_len++] = sizeof(u8) + sizeof(uuid) + data_len;
    73          eir[eir_len++] = EIR_SERVICE_DATA;
    74          put_unaligned_le16(uuid, &eir[eir_len]);
    75          eir_len += sizeof(uuid);
    76          memcpy(&eir[eir_len], data, data_len);
                            ^^^^^^^         ^^^^^^^^
    77          eir_len += data_len;
    78  
    79          return eir_len;
    80  }

The "eir" buffer has 252 bytes and data_len is 252 but we do a memcpy()
to &eir[4] so this can corrupt 4 bytes beyond the end of the buffer.

If you look back at the caller it sets conn->le_per_adv_data_len to a
max of 4 + 252 = 256 but truncated to 255.  This eventually gets passed
to iso_sock_getsockopt() leading to a read overflow.  But the first part
of the bug is in eir_append_service_data().

regards,
dan carpenter