Hi Gustavo,
On Sun, Feb 27, 2011 at 5:20 PM, Gustavo F. Padovan
<padovan@xxxxxxxxxxxxxx> wrote:
> Hi Vinicius,
>
> * Vinicius Costa Gomes <vinicius.gomes@xxxxxxxxxxxxx> [2011-02-21 14:23:51 -0300]:
>
>> This will allow using the crypto subsystem for encrypting data. As SMP
>> (Security Manager Protocol) is implemented almost entirely on the host
>> side and the crypto module already implements the needed methods
>> (AES-128), it makes sense to use it.
>>
>> This patch also adds a new Kconfig option to toggle the SMP support.
>>
>> Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@xxxxxxxxxxxxx>
>> Signed-off-by: Anderson Briglia <anderson.briglia@xxxxxxxxxxxxx>
>> ---
>> Âinclude/net/bluetooth/hci_core.h | Â Â2 ++
>> Ânet/bluetooth/Kconfig      Â|  Â6 ++++++
>> Ânet/bluetooth/hci_core.c     |  22 ++++++++++++++++++++++
>> Ânet/bluetooth/smp.c       Â|  17 +++++++++++++++--
>> Â4 files changed, 45 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
>> index d5d8454..e8dbde8 100644
>> --- a/include/net/bluetooth/hci_core.h
>> +++ b/include/net/bluetooth/hci_core.h
>> @@ -161,6 +161,8 @@ struct hci_dev {
>>
>> Â Â Â __u16 Â Â Â Â Â Â Â Â Â init_last_cmd;
>>
>> + Â Â struct crypto_blkcipher *tfm;
>> +
>>    struct inquiry_cache  Âinq_cache;
>>    struct hci_conn_hash  Âconn_hash;
>>    struct list_head    Âblacklist;
>> diff --git a/net/bluetooth/Kconfig b/net/bluetooth/Kconfig
>> index c6f9c2f..e9f40af 100644
>> --- a/net/bluetooth/Kconfig
>> +++ b/net/bluetooth/Kconfig
>> @@ -22,6 +22,7 @@ menuconfig BT
>> Â Â Â Â Â ÂBNEP Module (Bluetooth Network Encapsulation Protocol)
>> Â Â Â Â Â ÂCMTP Module (CAPI Message Transport Protocol)
>> Â Â Â Â Â ÂHIDP Module (Human Interface Device Protocol)
>> + Â Â Â Â ÂSMP Module (Security Manager Protocol)
>>
>> Â Â Â Â Say Y here to compile Bluetooth support into the kernel or say M to
>> Â Â Â Â compile it as module (bluetooth).
>> @@ -35,11 +36,16 @@ config BT_L2CAP
>> Â Â Â bool "L2CAP protocol support"
>> Â Â Â depends on BT
>> Â Â Â select CRC16
>> + Â Â select CRYPTO_BLKCIPHER
>> + Â Â select CRYPTO_AES
>> Â Â Â help
>> Â Â Â Â L2CAP (Logical Link Control and Adaptation Protocol) provides
>> Â Â Â Â connection oriented and connection-less data transport. ÂL2CAP
>> Â Â Â Â support is required for most Bluetooth applications.
>>
>> + Â Â Â Also included is support for SMP (Security Manager Protocol) which
>> + Â Â Â is the security layer on top of LE (Low Energy) links.
>> +
>> Âconfig BT_SCO
>> Â Â Â bool "SCO links support"
>> Â Â Â depends on BT
>> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
>> index b372fb8..ff67843 100644
>> --- a/net/bluetooth/hci_core.c
>> +++ b/net/bluetooth/hci_core.c
>> @@ -42,6 +42,7 @@
>> Â#include <linux/notifier.h>
>> Â#include <linux/rfkill.h>
>> Â#include <linux/timer.h>
>> +#include <linux/crypto.h>
>> Â#include <net/sock.h>
>>
>> Â#include <asm/system.h>
>> @@ -60,6 +61,8 @@ static void hci_notify(struct hci_dev *hdev, int event);
>>
>> Âstatic DEFINE_RWLOCK(hci_task_lock);
>>
>> +static int enable_smp;
>> +
>> Â/* HCI device list */
>> ÂLIST_HEAD(hci_dev_list);
>> ÂDEFINE_RWLOCK(hci_dev_list_lock);
>> @@ -1077,6 +1080,14 @@ static void hci_cmd_timer(unsigned long arg)
>> Â Â Â tasklet_schedule(&hdev->cmd_task);
>> Â}
>>
>> +static struct crypto_blkcipher *alloc_cypher(void)
>> +{
>> + Â Â if (enable_smp)
>> + Â Â Â Â Â Â return crypto_alloc_blkcipher("ecb(aes)", 0, CRYPTO_ALG_ASYNC);
>> +
>> + Â Â return ERR_PTR(-ENOTSUPP);
>> +}
>> +
>> Â/* Register HCI device */
>> Âint hci_register_dev(struct hci_dev *hdev)
>> Â{
>> @@ -1155,6 +1166,11 @@ int hci_register_dev(struct hci_dev *hdev)
>> Â Â Â if (!hdev->workqueue)
>> Â Â Â Â Â Â Â goto nomem;
>>
>> + Â Â hdev->tfm = alloc_cypher();
>> + Â Â if (IS_ERR(hdev->tfm))
>> + Â Â Â Â Â Â BT_INFO("Failed to load transform for ecb(aes): %ld",
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â PTR_ERR(hdev->tfm));
>> +
>> Â Â Â hci_register_sysfs(hdev);
>>
>> Â Â Â hdev->rfkill = rfkill_alloc(hdev->name, &hdev->dev,
>> @@ -1203,6 +1219,9 @@ int hci_unregister_dev(struct hci_dev *hdev)
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â !test_bit(HCI_SETUP, &hdev->flags))
>> Â Â Â Â Â Â Â mgmt_index_removed(hdev->id);
>>
>> + Â Â if (!IS_ERR(hdev->tfm))
>> + Â Â Â Â Â Â crypto_free_blkcipher(hdev->tfm);
>> +
>> Â Â Â hci_notify(hdev, HCI_DEV_UNREG);
>>
>> Â Â Â if (hdev->rfkill) {
>> @@ -2037,3 +2056,6 @@ static void hci_cmd_task(unsigned long arg)
>> Â Â Â Â Â Â Â }
>> Â Â Â }
>> Â}
>> +
>> +module_param(enable_smp, bool, 0644);
>> +MODULE_PARM_DESC(enable_smp, "Enable SMP support (LE only)");
>
> This all should be obviously inside smp.c
I have a couple of points against it:
1. it's only used for one purpose, it says whether to try or not to
allocate the block cypher, which is done during the adapter
registration;
2. If the current way is ok, it would mean that I would need to export
another method from smp.c, that was something that I tried to
minimize;
3. and my weakest argument, seems that there are other similar uses,
for example enable_mgmt.
But if you think the code will be clearer moving that to smp.c, will
be glad to change.
>
> Regards,
>
> --
> Gustavo F. Padovan
> http://profusion.mobi
>
Cheers,
--
Vinicius
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
