From: Dan Carpenter <dan.carpenter@oracle.com>
To: Lin Horse <kylin.formalin@gmail.com>,
Luiz Augusto von Dentz <luiz.von.dentz@intel.com>,
Marcel Holtmann <marcel@holtmann.org>
Cc: Greg KH <greg@kroah.com>,
security@kernel.org, linux-bluetooth@vger.kernel.org,
netdev@vger.kernel.org, Johannes Berg <johannes@sipsolutions.net>
Subject: Re: OOB Read in hci_cc_read_local_name() cause information leak
Date: Tue, 22 Jun 2021 13:32:32 +0300 [thread overview]
Message-ID: <20210622103232.GL1901@kadam> (raw)
In-Reply-To: <20210521150454.GD24442@kadam>
Hi Luiz,
I was looking through old bug reports that Lin Horse had reported and
was wondering when your patchset would be applied?
https://lore.kernel.org/linux-bluetooth/20210419171257.3865181-1-luiz.dentz@gmail.com/
I really like the hci_skb_pull() function and I think that it should be
made into a top level function which all drivers can use. It's like
skb_pull() but it returns the old skb->data instead of skb->data + len.
The skb_pull() still updates skb->data and skb->len.
static void *skb_pull_data(struct sk_buff *skb, size_t len)
{
void *data = skb->data;
if (skb->len < len)
return NULL;
skb_pull(skb, len);
return data;
}
There is a lot of code that does "struct foo *p = (void *)skb->data;"
and that's hard to audit and error prone. Changing it to:
p = skb_pull_data(skb, sizeof(*p));
if (!p)
return;
seems more clear and safe.
regards,
dan carpenter
> net/bluetooth/hci_event.c:119 hci_cc_role_discovery() assignment assumes 'skb->len' is '4' bytes
> net/bluetooth/hci_event.c:138 hci_cc_read_link_policy() assignment assumes 'skb->len' is '5' bytes
> net/bluetooth/hci_event.c:157 hci_cc_write_link_policy() assignment assumes 'skb->len' is '3' bytes
> net/bluetooth/hci_event.c:182 hci_cc_read_def_link_policy() assignment assumes 'skb->len' is '3' bytes
> net/bluetooth/hci_event.c:246 hci_cc_read_stored_link_key() assignment assumes 'skb->len' is '3' bytes
> net/bluetooth/hci_event.c:264 hci_cc_delete_stored_link_key() assignment assumes 'skb->len' is '2' bytes
> net/bluetooth/hci_event.c:300 hci_cc_read_local_name() assignment assumes 'skb->len' is '249' bytes
> net/bluetooth/hci_event.c:423 hci_cc_read_class_of_dev() assignment assumes 'skb->len' is '4' bytes
> net/bluetooth/hci_event.c:460 hci_cc_read_voice_setting() assignment assumes 'skb->len' is '3' bytes
> net/bluetooth/hci_event.c:513 hci_cc_read_num_supported_iac() assignment assumes 'skb->len' is '2' bytes
> net/bluetooth/hci_event.c:589 hci_cc_read_local_version() assignment assumes 'skb->len' is '9' bytes
> net/bluetooth/hci_event.c:609 hci_cc_read_local_commands() assignment assumes 'skb->len' is '65' bytes
> net/bluetooth/hci_event.c:624 hci_cc_read_auth_payload_timeout() assignment assumes 'skb->len' is '5' bytes
> net/bluetooth/hci_event.c:644 hci_cc_write_auth_payload_timeout() assignment assumes 'skb->len' is '3' bytes
> net/bluetooth/hci_event.c:669 hci_cc_read_local_features() assignment assumes 'skb->len' is '9' bytes
> net/bluetooth/hci_event.c:719 hci_cc_read_local_ext_features() assignment assumes 'skb->len' is '11' bytes
> net/bluetooth/hci_event.c:736 hci_cc_read_flow_control_mode() assignment assumes 'skb->len' is '2' bytes
> net/bluetooth/hci_event.c:748 hci_cc_read_buffer_size() assignment assumes 'skb->len' is '8' bytes
> net/bluetooth/hci_event.c:774 hci_cc_read_bd_addr() assignment assumes 'skb->len' is '7' bytes
> net/bluetooth/hci_event.c:791 hci_cc_read_local_pairing_opts() assignment assumes 'skb->len' is '3' bytes
> net/bluetooth/hci_event.c:808 hci_cc_read_page_scan_activity() assignment assumes 'skb->len' is '5' bytes
> net/bluetooth/hci_event.c:843 hci_cc_read_page_scan_type() assignment assumes 'skb->len' is '2' bytes
> net/bluetooth/hci_event.c:873 hci_cc_read_data_block_size() assignment assumes 'skb->len' is '7' bytes
> net/bluetooth/hci_event.c:892 hci_cc_read_clock() assignment assumes 'skb->len' is '9' bytes
> net/bluetooth/hci_event.c:928 hci_cc_read_local_amp_info() assignment assumes 'skb->len' is '31' bytes
> net/bluetooth/hci_event.c:950 hci_cc_read_inq_rsp_tx_power() assignment assumes 'skb->len' is '2' bytes
> net/bluetooth/hci_event.c:963 hci_cc_read_def_err_data_reporting() assignment assumes 'skb->len' is '2' bytes
> net/bluetooth/hci_event.c:993 hci_cc_pin_code_reply() assignment assumes 'skb->len' is '7' bytes
> net/bluetooth/hci_event.c:1021 hci_cc_pin_code_neg_reply() assignment assumes 'skb->len' is '7' bytes
> net/bluetooth/hci_event.c:1037 hci_cc_le_read_buffer_size() assignment assumes 'skb->len' is '4' bytes
> net/bluetooth/hci_event.c:1055 hci_cc_le_read_local_features() assignment assumes 'skb->len' is '9' bytes
> net/bluetooth/hci_event.c:1068 hci_cc_le_read_adv_tx_power() assignment assumes 'skb->len' is '2' bytes
> net/bluetooth/hci_event.c:1080 hci_cc_user_confirm_reply() assignment assumes 'skb->len' is '7' bytes
> net/bluetooth/hci_event.c:1096 hci_cc_user_confirm_neg_reply() assignment assumes 'skb->len' is '7' bytes
> net/bluetooth/hci_event.c:1111 hci_cc_user_passkey_reply() assignment assumes 'skb->len' is '7' bytes
> net/bluetooth/hci_event.c:1127 hci_cc_user_passkey_neg_reply() assignment assumes 'skb->len' is '7' bytes
> net/bluetooth/hci_event.c:1143 hci_cc_read_local_oob_data() assignment assumes 'skb->len' is '33' bytes
> net/bluetooth/hci_event.c:1151 hci_cc_read_local_oob_ext_data() assignment assumes 'skb->len' is '65' bytes
> net/bluetooth/hci_event.c:1230 hci_cc_le_read_transmit_power() assignment assumes 'skb->len' is '3' bytes
> net/bluetooth/hci_event.c:1484 hci_cc_le_read_num_adv_sets() assignment assumes 'skb->len' is '2' bytes
> net/bluetooth/hci_event.c:1498 hci_cc_le_read_white_list_size() assignment assumes 'skb->len' is '2' bytes
> net/bluetooth/hci_event.c:1562 hci_cc_le_read_supported_states() assignment assumes 'skb->len' is '9' bytes
> net/bluetooth/hci_event.c:1575 hci_cc_le_read_def_data_len() assignment assumes 'skb->len' is '5' bytes
> net/bluetooth/hci_event.c:1660 hci_cc_le_read_resolv_list_size() assignment assumes 'skb->len' is '2' bytes
> net/bluetooth/hci_event.c:1697 hci_cc_le_read_max_data_len() assignment assumes 'skb->len' is '9' bytes
> net/bluetooth/hci_event.c:1765 hci_cc_set_ext_adv_param() assignment assumes 'skb->len' is '2' bytes
> net/bluetooth/hci_event.c:1796 hci_cc_read_rssi() assignment assumes 'skb->len' is '4' bytes
> net/bluetooth/hci_event.c:1816 hci_cc_read_tx_power() assignment assumes 'skb->len' is '4' bytes
> net/bluetooth/hci_event.c:2607 hci_conn_complete_evt() assignment assumes 'skb->len' is '11' bytes
> net/bluetooth/hci_event.c:2731 hci_conn_request_evt() assignment assumes 'skb->len' is '10' bytes
> net/bluetooth/hci_event.c:2842 hci_disconn_complete_evt() assignment assumes 'skb->len' is '4' bytes
> net/bluetooth/hci_event.c:2934 hci_auth_complete_evt() assignment assumes 'skb->len' is '3' bytes
> net/bluetooth/hci_event.c:3004 hci_remote_name_evt() assignment assumes 'skb->len' is '255' bytes
> net/bluetooth/hci_event.c:3087 hci_encrypt_change_evt() assignment assumes 'skb->len' is '4' bytes
> net/bluetooth/hci_event.c:3202 hci_change_link_key_complete_evt() assignment assumes 'skb->len' is '3' bytes
> net/bluetooth/hci_event.c:3225 hci_remote_features_evt() assignment assumes 'skb->len' is '11' bytes
> net/bluetooth/hci_event.c:3293 hci_cmd_complete_evt() assignment assumes 'skb->len' is '3' bytes
> net/bluetooth/hci_event.c:3670 hci_cmd_status_evt() assignment assumes 'skb->len' is '4' bytes
> net/bluetooth/hci_event.c:3776 hci_hardware_error_evt() assignment assumes 'skb->len' is '1' bytes
> net/bluetooth/hci_event.c:3785 hci_role_change_evt() assignment assumes 'skb->len' is '8' bytes
> net/bluetooth/hci_event.c:3807 hci_num_comp_pkts_evt() assignment assumes 'skb->len' is '1' bytes
> net/bluetooth/hci_event.c:3895 hci_num_comp_blocks_evt() assignment assumes 'skb->len' is '3' bytes
> net/bluetooth/hci_event.c:3946 hci_mode_change_evt() assignment assumes 'skb->len' is '6' bytes
> net/bluetooth/hci_event.c:3974 hci_pin_code_request_evt() assignment assumes 'skb->len' is '6' bytes
> net/bluetooth/hci_event.c:4044 hci_link_key_request_evt() assignment assumes 'skb->len' is '6' bytes
> net/bluetooth/hci_event.c:4104 hci_link_key_notify_evt() assignment assumes 'skb->len' is '23' bytes
> net/bluetooth/hci_event.c:4164 hci_clock_offset_evt() assignment assumes 'skb->len' is '5' bytes
> net/bluetooth/hci_event.c:4187 hci_pkt_type_change_evt() assignment assumes 'skb->len' is '5' bytes
> net/bluetooth/hci_event.c:4203 hci_pscan_rep_mode_evt() assignment assumes 'skb->len' is '7' bytes
> net/bluetooth/hci_event.c:4293 hci_remote_ext_features_evt() assignment assumes 'skb->len' is '13' bytes
> net/bluetooth/hci_event.c:4357 hci_sync_conn_complete_evt() assignment assumes 'skb->len' is '17' bytes
> net/bluetooth/hci_event.c:4505 hci_key_refresh_complete_evt() assignment assumes 'skb->len' is '3' bytes
> net/bluetooth/hci_event.c:4614 hci_io_capa_request_evt() assignment assumes 'skb->len' is '6' bytes
> net/bluetooth/hci_event.c:4683 hci_io_capa_reply_evt() assignment assumes 'skb->len' is '9' bytes
> net/bluetooth/hci_event.c:4704 hci_user_confirm_request_evt() assignment assumes 'skb->len' is '10' bytes
> net/bluetooth/hci_event.c:4789 hci_user_passkey_request_evt() assignment assumes 'skb->len' is '6' bytes
> net/bluetooth/hci_event.c:4800 hci_user_passkey_notify_evt() assignment assumes 'skb->len' is '10' bytes
> net/bluetooth/hci_event.c:4820 hci_keypress_notify_evt() assignment assumes 'skb->len' is '7' bytes
> net/bluetooth/hci_event.c:4859 hci_simple_pair_complete_evt() assignment assumes 'skb->len' is '7' bytes
> net/bluetooth/hci_event.c:4890 hci_remote_host_features_evt() assignment assumes 'skb->len' is '14' bytes
> net/bluetooth/hci_event.c:4912 hci_remote_oob_data_request_evt() assignment assumes 'skb->len' is '6' bytes
> net/bluetooth/hci_event.c:4966 hci_chan_selected_evt() assignment assumes 'skb->len' is '1' bytes
> net/bluetooth/hci_event.c:4983 hci_phy_link_complete_evt() assignment assumes 'skb->len' is '2' bytes
> net/bluetooth/hci_event.c:5023 hci_loglink_complete_evt() assignment assumes 'skb->len' is '5' bytes
> net/bluetooth/hci_event.c:5063 hci_disconn_loglink_complete_evt() assignment assumes 'skb->len' is '4' bytes
> net/bluetooth/hci_event.c:5087 hci_disconn_phylink_complete_evt() assignment assumes 'skb->len' is '3' bytes
> net/bluetooth/hci_event.c:5271 hci_le_conn_complete_evt() assignment assumes 'skb->len' is '18' bytes
> net/bluetooth/hci_event.c:5285 hci_le_enh_conn_complete_evt() assignment assumes 'skb->len' is '30' bytes
> net/bluetooth/hci_event.c:5303 hci_le_ext_adv_term_evt() assignment assumes 'skb->len' is '5' bytes
> net/bluetooth/hci_event.c:5332 hci_le_conn_update_complete_evt() assignment assumes 'skb->len' is '9' bytes
> net/bluetooth/hci_event.c:5749 hci_le_remote_feat_complete_evt() assignment assumes 'skb->len' is '11' bytes
> net/bluetooth/hci_event.c:5790 hci_le_ltk_request_evt() assignment assumes 'skb->len' is '12' bytes
> net/bluetooth/hci_event.c:5867 hci_le_remote_conn_param_req_evt() assignment assumes 'skb->len' is '10' bytes
> net/bluetooth/hci_event.c:5944 hci_le_phy_update_evt() assignment assumes 'skb->len' is '5' bytes
> net/bluetooth/hci_event.c:5967 hci_le_meta_evt() assignment assumes 'skb->len' is '1' bytes
> net/bluetooth/hci_event.c:6079 hci_store_wake_reason() assignment assumes 'skb->len' is '11' bytes
> net/bluetooth/hci_event.c:6080 hci_store_wake_reason() assignment assumes 'skb->len' is '10' bytes
> net/bluetooth/hci_event.c:6106 hci_store_wake_reason() assignment assumes 'skb->len' is '1' bytes
> net/bluetooth/hci_event.c:6144 hci_event_packet() assignment assumes 'skb->len' is '2' bytes
> net/bluetooth/hci_event.c:6157 hci_event_packet() assignment assumes 'hdev->sent_cmd->len' is '3' bytes
>
> Also these warnings are probably worth looking into:
>
> net/bluetooth/hci_event.c:3823 hci_num_comp_pkts_evt() warn: uncapped user loop: 'ev->num_hndl'
> net/bluetooth/hci_event.c:3912 hci_num_comp_blocks_evt() warn: uncapped user loop: 'ev->num_hndl'
> net/bluetooth/hci_event.c:5656 hci_le_adv_report_evt() warn: uncapped user loop: 'num_reports--'
> net/bluetooth/hci_event.c:5726 hci_le_ext_adv_report_evt() warn: uncapped user loop: 'num_reports--'
next parent reply other threads:[~2021-06-22 10:33 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CAJjojJvCv2aMWt_cjSC2eBNBDGwV2ottSApgmHdJmup6-+_k4w@mail.gmail.com>
[not found] ` <YKc6SloWBg5maJpU@kroah.com>
[not found] ` <CAJjojJtvYeK4N7E8MZkF8YBbp-mvjzoeJgjb=6zQB6h-5tRkNg@mail.gmail.com>
[not found] ` <20210521090256.GA24442@kadam>
[not found] ` <CAJjojJu6ZaAZOs1K=OsvA0=+ZNVATdT3YgbsqSzTYeFJFCgqzQ@mail.gmail.com>
[not found] ` <20210521150454.GD24442@kadam>
2021-06-22 10:32 ` Dan Carpenter [this message]
2021-06-22 18:11 ` OOB Read in hci_cc_read_local_name() cause information leak Luiz Augusto von Dentz
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=20210622103232.GL1901@kadam \
--to=dan.carpenter@oracle.com \
--cc=greg@kroah.com \
--cc=johannes@sipsolutions.net \
--cc=kylin.formalin@gmail.com \
--cc=linux-bluetooth@vger.kernel.org \
--cc=luiz.von.dentz@intel.com \
--cc=marcel@holtmann.org \
--cc=netdev@vger.kernel.org \
--cc=security@kernel.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).