* [PATCH v2] Bluetooth: Move force_bredr_smp debugfs into hci_debugfs_create_bredr
@ 2020-09-29 8:03 Claire Chang
2020-10-07 4:38 ` Claire Chang
2020-11-09 12:57 ` Marcel Holtmann
0 siblings, 2 replies; 5+ messages in thread
From: Claire Chang @ 2020-09-29 8:03 UTC (permalink / raw)
To: marcel, johan.hedberg, davem, kuba
Cc: netdev, linux-bluetooth, linux-kernel, Claire Chang
Avoid multiple attempts to create the debugfs entry, force_bredr_smp,
by moving it from the SMP registration to the BR/EDR controller init
section. hci_debugfs_create_bredr is only called when HCI_SETUP and
HCI_CONFIG is not set.
Signed-off-by: Claire Chang <tientzu@chromium.org>
---
v2: correct a typo in commit message
net/bluetooth/hci_debugfs.c | 50 +++++++++++++++++++++++++++++++++++++
net/bluetooth/smp.c | 44 ++------------------------------
net/bluetooth/smp.h | 2 ++
3 files changed, 54 insertions(+), 42 deletions(-)
diff --git a/net/bluetooth/hci_debugfs.c b/net/bluetooth/hci_debugfs.c
index 5e8af2658e44..4626e0289a97 100644
--- a/net/bluetooth/hci_debugfs.c
+++ b/net/bluetooth/hci_debugfs.c
@@ -494,6 +494,45 @@ static int auto_accept_delay_get(void *data, u64 *val)
DEFINE_SIMPLE_ATTRIBUTE(auto_accept_delay_fops, auto_accept_delay_get,
auto_accept_delay_set, "%llu\n");
+static ssize_t force_bredr_smp_read(struct file *file,
+ char __user *user_buf,
+ size_t count, loff_t *ppos)
+{
+ struct hci_dev *hdev = file->private_data;
+ char buf[3];
+
+ buf[0] = hci_dev_test_flag(hdev, HCI_FORCE_BREDR_SMP) ? 'Y' : 'N';
+ buf[1] = '\n';
+ buf[2] = '\0';
+ return simple_read_from_buffer(user_buf, count, ppos, buf, 2);
+}
+
+static ssize_t force_bredr_smp_write(struct file *file,
+ const char __user *user_buf,
+ size_t count, loff_t *ppos)
+{
+ struct hci_dev *hdev = file->private_data;
+ bool enable;
+ int err;
+
+ err = kstrtobool_from_user(user_buf, count, &enable);
+ if (err)
+ return err;
+
+ err = smp_force_bredr(hdev, enable);
+ if (err)
+ return err;
+
+ return count;
+}
+
+static const struct file_operations force_bredr_smp_fops = {
+ .open = simple_open,
+ .read = force_bredr_smp_read,
+ .write = force_bredr_smp_write,
+ .llseek = default_llseek,
+};
+
static int idle_timeout_set(void *data, u64 val)
{
struct hci_dev *hdev = data;
@@ -589,6 +628,17 @@ void hci_debugfs_create_bredr(struct hci_dev *hdev)
debugfs_create_file("voice_setting", 0444, hdev->debugfs, hdev,
&voice_setting_fops);
+ /* If the controller does not support BR/EDR Secure Connections
+ * feature, then the BR/EDR SMP channel shall not be present.
+ *
+ * To test this with Bluetooth 4.0 controllers, create a debugfs
+ * switch that allows forcing BR/EDR SMP support and accepting
+ * cross-transport pairing on non-AES encrypted connections.
+ */
+ if (!lmp_sc_capable(hdev))
+ debugfs_create_file("force_bredr_smp", 0644, hdev->debugfs,
+ hdev, &force_bredr_smp_fops);
+
if (lmp_ssp_capable(hdev)) {
debugfs_create_file("ssp_debug_mode", 0444, hdev->debugfs,
hdev, &ssp_debug_mode_fops);
diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
index 433227f96c73..8b817e4358fd 100644
--- a/net/bluetooth/smp.c
+++ b/net/bluetooth/smp.c
@@ -3353,31 +3353,8 @@ static void smp_del_chan(struct l2cap_chan *chan)
l2cap_chan_put(chan);
}
-static ssize_t force_bredr_smp_read(struct file *file,
- char __user *user_buf,
- size_t count, loff_t *ppos)
+int smp_force_bredr(struct hci_dev *hdev, bool enable)
{
- struct hci_dev *hdev = file->private_data;
- char buf[3];
-
- buf[0] = hci_dev_test_flag(hdev, HCI_FORCE_BREDR_SMP) ? 'Y': 'N';
- buf[1] = '\n';
- buf[2] = '\0';
- return simple_read_from_buffer(user_buf, count, ppos, buf, 2);
-}
-
-static ssize_t force_bredr_smp_write(struct file *file,
- const char __user *user_buf,
- size_t count, loff_t *ppos)
-{
- struct hci_dev *hdev = file->private_data;
- bool enable;
- int err;
-
- err = kstrtobool_from_user(user_buf, count, &enable);
- if (err)
- return err;
-
if (enable == hci_dev_test_flag(hdev, HCI_FORCE_BREDR_SMP))
return -EALREADY;
@@ -3399,16 +3376,9 @@ static ssize_t force_bredr_smp_write(struct file *file,
hci_dev_change_flag(hdev, HCI_FORCE_BREDR_SMP);
- return count;
+ return 0;
}
-static const struct file_operations force_bredr_smp_fops = {
- .open = simple_open,
- .read = force_bredr_smp_read,
- .write = force_bredr_smp_write,
- .llseek = default_llseek,
-};
-
int smp_register(struct hci_dev *hdev)
{
struct l2cap_chan *chan;
@@ -3433,17 +3403,7 @@ int smp_register(struct hci_dev *hdev)
hdev->smp_data = chan;
- /* If the controller does not support BR/EDR Secure Connections
- * feature, then the BR/EDR SMP channel shall not be present.
- *
- * To test this with Bluetooth 4.0 controllers, create a debugfs
- * switch that allows forcing BR/EDR SMP support and accepting
- * cross-transport pairing on non-AES encrypted connections.
- */
if (!lmp_sc_capable(hdev)) {
- debugfs_create_file("force_bredr_smp", 0644, hdev->debugfs,
- hdev, &force_bredr_smp_fops);
-
/* Flag can be already set here (due to power toggle) */
if (!hci_dev_test_flag(hdev, HCI_FORCE_BREDR_SMP))
return 0;
diff --git a/net/bluetooth/smp.h b/net/bluetooth/smp.h
index 121edadd5f8d..fc35a8bf358e 100644
--- a/net/bluetooth/smp.h
+++ b/net/bluetooth/smp.h
@@ -193,6 +193,8 @@ bool smp_irk_matches(struct hci_dev *hdev, const u8 irk[16],
int smp_generate_rpa(struct hci_dev *hdev, const u8 irk[16], bdaddr_t *rpa);
int smp_generate_oob(struct hci_dev *hdev, u8 hash[16], u8 rand[16]);
+int smp_force_bredr(struct hci_dev *hdev, bool enable);
+
int smp_register(struct hci_dev *hdev);
void smp_unregister(struct hci_dev *hdev);
--
2.28.0.618.gf4bc123cb7-goog
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] Bluetooth: Move force_bredr_smp debugfs into hci_debugfs_create_bredr
2020-09-29 8:03 [PATCH v2] Bluetooth: Move force_bredr_smp debugfs into hci_debugfs_create_bredr Claire Chang
@ 2020-10-07 4:38 ` Claire Chang
2020-10-27 15:49 ` Alain Michaud
2020-11-09 12:57 ` Marcel Holtmann
1 sibling, 1 reply; 5+ messages in thread
From: Claire Chang @ 2020-10-07 4:38 UTC (permalink / raw)
To: marcel, johan.hedberg, davem, kuba; +Cc: netdev, BlueZ, lkml
Hi,
This patch is to fix the kernel error
[ 46.271811] debugfs: File 'force_bredr_smp' in directory 'hci0'
already present!
When powering off and on the bluetooth, the smp_register will try to create the
force_bredr_smp entry again.
Move the creation to hci_debugfs_create_bredr so the force_bredr_smp entry will
only be created when HCI_SETUP and HCI_CONFIG are not set.
Thanks,
Claire
On Tue, Sep 29, 2020 at 4:03 PM Claire Chang <tientzu@chromium.org> wrote:
>
> Avoid multiple attempts to create the debugfs entry, force_bredr_smp,
> by moving it from the SMP registration to the BR/EDR controller init
> section. hci_debugfs_create_bredr is only called when HCI_SETUP and
> HCI_CONFIG is not set.
>
> Signed-off-by: Claire Chang <tientzu@chromium.org>
> ---
> v2: correct a typo in commit message
>
> net/bluetooth/hci_debugfs.c | 50 +++++++++++++++++++++++++++++++++++++
> net/bluetooth/smp.c | 44 ++------------------------------
> net/bluetooth/smp.h | 2 ++
> 3 files changed, 54 insertions(+), 42 deletions(-)
>
> diff --git a/net/bluetooth/hci_debugfs.c b/net/bluetooth/hci_debugfs.c
> index 5e8af2658e44..4626e0289a97 100644
> --- a/net/bluetooth/hci_debugfs.c
> +++ b/net/bluetooth/hci_debugfs.c
> @@ -494,6 +494,45 @@ static int auto_accept_delay_get(void *data, u64 *val)
> DEFINE_SIMPLE_ATTRIBUTE(auto_accept_delay_fops, auto_accept_delay_get,
> auto_accept_delay_set, "%llu\n");
>
> +static ssize_t force_bredr_smp_read(struct file *file,
> + char __user *user_buf,
> + size_t count, loff_t *ppos)
> +{
> + struct hci_dev *hdev = file->private_data;
> + char buf[3];
> +
> + buf[0] = hci_dev_test_flag(hdev, HCI_FORCE_BREDR_SMP) ? 'Y' : 'N';
> + buf[1] = '\n';
> + buf[2] = '\0';
> + return simple_read_from_buffer(user_buf, count, ppos, buf, 2);
> +}
> +
> +static ssize_t force_bredr_smp_write(struct file *file,
> + const char __user *user_buf,
> + size_t count, loff_t *ppos)
> +{
> + struct hci_dev *hdev = file->private_data;
> + bool enable;
> + int err;
> +
> + err = kstrtobool_from_user(user_buf, count, &enable);
> + if (err)
> + return err;
> +
> + err = smp_force_bredr(hdev, enable);
> + if (err)
> + return err;
> +
> + return count;
> +}
> +
> +static const struct file_operations force_bredr_smp_fops = {
> + .open = simple_open,
> + .read = force_bredr_smp_read,
> + .write = force_bredr_smp_write,
> + .llseek = default_llseek,
> +};
> +
> static int idle_timeout_set(void *data, u64 val)
> {
> struct hci_dev *hdev = data;
> @@ -589,6 +628,17 @@ void hci_debugfs_create_bredr(struct hci_dev *hdev)
> debugfs_create_file("voice_setting", 0444, hdev->debugfs, hdev,
> &voice_setting_fops);
>
> + /* If the controller does not support BR/EDR Secure Connections
> + * feature, then the BR/EDR SMP channel shall not be present.
> + *
> + * To test this with Bluetooth 4.0 controllers, create a debugfs
> + * switch that allows forcing BR/EDR SMP support and accepting
> + * cross-transport pairing on non-AES encrypted connections.
> + */
> + if (!lmp_sc_capable(hdev))
> + debugfs_create_file("force_bredr_smp", 0644, hdev->debugfs,
> + hdev, &force_bredr_smp_fops);
> +
> if (lmp_ssp_capable(hdev)) {
> debugfs_create_file("ssp_debug_mode", 0444, hdev->debugfs,
> hdev, &ssp_debug_mode_fops);
> diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
> index 433227f96c73..8b817e4358fd 100644
> --- a/net/bluetooth/smp.c
> +++ b/net/bluetooth/smp.c
> @@ -3353,31 +3353,8 @@ static void smp_del_chan(struct l2cap_chan *chan)
> l2cap_chan_put(chan);
> }
>
> -static ssize_t force_bredr_smp_read(struct file *file,
> - char __user *user_buf,
> - size_t count, loff_t *ppos)
> +int smp_force_bredr(struct hci_dev *hdev, bool enable)
> {
> - struct hci_dev *hdev = file->private_data;
> - char buf[3];
> -
> - buf[0] = hci_dev_test_flag(hdev, HCI_FORCE_BREDR_SMP) ? 'Y': 'N';
> - buf[1] = '\n';
> - buf[2] = '\0';
> - return simple_read_from_buffer(user_buf, count, ppos, buf, 2);
> -}
> -
> -static ssize_t force_bredr_smp_write(struct file *file,
> - const char __user *user_buf,
> - size_t count, loff_t *ppos)
> -{
> - struct hci_dev *hdev = file->private_data;
> - bool enable;
> - int err;
> -
> - err = kstrtobool_from_user(user_buf, count, &enable);
> - if (err)
> - return err;
> -
> if (enable == hci_dev_test_flag(hdev, HCI_FORCE_BREDR_SMP))
> return -EALREADY;
>
> @@ -3399,16 +3376,9 @@ static ssize_t force_bredr_smp_write(struct file *file,
>
> hci_dev_change_flag(hdev, HCI_FORCE_BREDR_SMP);
>
> - return count;
> + return 0;
> }
>
> -static const struct file_operations force_bredr_smp_fops = {
> - .open = simple_open,
> - .read = force_bredr_smp_read,
> - .write = force_bredr_smp_write,
> - .llseek = default_llseek,
> -};
> -
> int smp_register(struct hci_dev *hdev)
> {
> struct l2cap_chan *chan;
> @@ -3433,17 +3403,7 @@ int smp_register(struct hci_dev *hdev)
>
> hdev->smp_data = chan;
>
> - /* If the controller does not support BR/EDR Secure Connections
> - * feature, then the BR/EDR SMP channel shall not be present.
> - *
> - * To test this with Bluetooth 4.0 controllers, create a debugfs
> - * switch that allows forcing BR/EDR SMP support and accepting
> - * cross-transport pairing on non-AES encrypted connections.
> - */
> if (!lmp_sc_capable(hdev)) {
> - debugfs_create_file("force_bredr_smp", 0644, hdev->debugfs,
> - hdev, &force_bredr_smp_fops);
> -
> /* Flag can be already set here (due to power toggle) */
> if (!hci_dev_test_flag(hdev, HCI_FORCE_BREDR_SMP))
> return 0;
> diff --git a/net/bluetooth/smp.h b/net/bluetooth/smp.h
> index 121edadd5f8d..fc35a8bf358e 100644
> --- a/net/bluetooth/smp.h
> +++ b/net/bluetooth/smp.h
> @@ -193,6 +193,8 @@ bool smp_irk_matches(struct hci_dev *hdev, const u8 irk[16],
> int smp_generate_rpa(struct hci_dev *hdev, const u8 irk[16], bdaddr_t *rpa);
> int smp_generate_oob(struct hci_dev *hdev, u8 hash[16], u8 rand[16]);
>
> +int smp_force_bredr(struct hci_dev *hdev, bool enable);
> +
> int smp_register(struct hci_dev *hdev);
> void smp_unregister(struct hci_dev *hdev);
>
> --
> 2.28.0.618.gf4bc123cb7-goog
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] Bluetooth: Move force_bredr_smp debugfs into hci_debugfs_create_bredr
2020-10-07 4:38 ` Claire Chang
@ 2020-10-27 15:49 ` Alain Michaud
2020-10-27 17:17 ` Luiz Augusto von Dentz
0 siblings, 1 reply; 5+ messages in thread
From: Alain Michaud @ 2020-10-27 15:49 UTC (permalink / raw)
To: Claire Chang
Cc: Marcel Holtmann, Johan Hedberg, David S. Miller, Jakub Kicinski,
netdev, BlueZ, lkml
Friendly ping and adding my review-by tag.
On Wed, Oct 7, 2020 at 12:38 AM Claire Chang <tientzu@chromium.org> wrote:
>
> Hi,
>
> This patch is to fix the kernel error
> [ 46.271811] debugfs: File 'force_bredr_smp' in directory 'hci0'
> already present!
>
> When powering off and on the bluetooth, the smp_register will try to create the
> force_bredr_smp entry again.
> Move the creation to hci_debugfs_create_bredr so the force_bredr_smp entry will
> only be created when HCI_SETUP and HCI_CONFIG are not set.
>
> Thanks,
> Claire
>
> On Tue, Sep 29, 2020 at 4:03 PM Claire Chang <tientzu@chromium.org> wrote:
> >
> > Avoid multiple attempts to create the debugfs entry, force_bredr_smp,
> > by moving it from the SMP registration to the BR/EDR controller init
> > section. hci_debugfs_create_bredr is only called when HCI_SETUP and
> > HCI_CONFIG is not set.
> >
> > Signed-off-by: Claire Chang <tientzu@chromium.org>
Reviewed-by: Alain Michaud <alainm@chromium.org>
> > ---
> > v2: correct a typo in commit message
> >
> > net/bluetooth/hci_debugfs.c | 50 +++++++++++++++++++++++++++++++++++++
> > net/bluetooth/smp.c | 44 ++------------------------------
> > net/bluetooth/smp.h | 2 ++
> > 3 files changed, 54 insertions(+), 42 deletions(-)
> >
> > diff --git a/net/bluetooth/hci_debugfs.c b/net/bluetooth/hci_debugfs.c
> > index 5e8af2658e44..4626e0289a97 100644
> > --- a/net/bluetooth/hci_debugfs.c
> > +++ b/net/bluetooth/hci_debugfs.c
> > @@ -494,6 +494,45 @@ static int auto_accept_delay_get(void *data, u64 *val)
> > DEFINE_SIMPLE_ATTRIBUTE(auto_accept_delay_fops, auto_accept_delay_get,
> > auto_accept_delay_set, "%llu\n");
> >
> > +static ssize_t force_bredr_smp_read(struct file *file,
> > + char __user *user_buf,
> > + size_t count, loff_t *ppos)
> > +{
> > + struct hci_dev *hdev = file->private_data;
> > + char buf[3];
> > +
> > + buf[0] = hci_dev_test_flag(hdev, HCI_FORCE_BREDR_SMP) ? 'Y' : 'N';
> > + buf[1] = '\n';
> > + buf[2] = '\0';
> > + return simple_read_from_buffer(user_buf, count, ppos, buf, 2);
> > +}
> > +
> > +static ssize_t force_bredr_smp_write(struct file *file,
> > + const char __user *user_buf,
> > + size_t count, loff_t *ppos)
> > +{
> > + struct hci_dev *hdev = file->private_data;
> > + bool enable;
> > + int err;
> > +
> > + err = kstrtobool_from_user(user_buf, count, &enable);
> > + if (err)
> > + return err;
> > +
> > + err = smp_force_bredr(hdev, enable);
> > + if (err)
> > + return err;
> > +
> > + return count;
> > +}
> > +
> > +static const struct file_operations force_bredr_smp_fops = {
> > + .open = simple_open,
> > + .read = force_bredr_smp_read,
> > + .write = force_bredr_smp_write,
> > + .llseek = default_llseek,
> > +};
> > +
> > static int idle_timeout_set(void *data, u64 val)
> > {
> > struct hci_dev *hdev = data;
> > @@ -589,6 +628,17 @@ void hci_debugfs_create_bredr(struct hci_dev *hdev)
> > debugfs_create_file("voice_setting", 0444, hdev->debugfs, hdev,
> > &voice_setting_fops);
> >
> > + /* If the controller does not support BR/EDR Secure Connections
> > + * feature, then the BR/EDR SMP channel shall not be present.
> > + *
> > + * To test this with Bluetooth 4.0 controllers, create a debugfs
> > + * switch that allows forcing BR/EDR SMP support and accepting
> > + * cross-transport pairing on non-AES encrypted connections.
> > + */
> > + if (!lmp_sc_capable(hdev))
> > + debugfs_create_file("force_bredr_smp", 0644, hdev->debugfs,
> > + hdev, &force_bredr_smp_fops);
> > +
> > if (lmp_ssp_capable(hdev)) {
> > debugfs_create_file("ssp_debug_mode", 0444, hdev->debugfs,
> > hdev, &ssp_debug_mode_fops);
> > diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
> > index 433227f96c73..8b817e4358fd 100644
> > --- a/net/bluetooth/smp.c
> > +++ b/net/bluetooth/smp.c
> > @@ -3353,31 +3353,8 @@ static void smp_del_chan(struct l2cap_chan *chan)
> > l2cap_chan_put(chan);
> > }
> >
> > -static ssize_t force_bredr_smp_read(struct file *file,
> > - char __user *user_buf,
> > - size_t count, loff_t *ppos)
> > +int smp_force_bredr(struct hci_dev *hdev, bool enable)
> > {
> > - struct hci_dev *hdev = file->private_data;
> > - char buf[3];
> > -
> > - buf[0] = hci_dev_test_flag(hdev, HCI_FORCE_BREDR_SMP) ? 'Y': 'N';
> > - buf[1] = '\n';
> > - buf[2] = '\0';
> > - return simple_read_from_buffer(user_buf, count, ppos, buf, 2);
> > -}
> > -
> > -static ssize_t force_bredr_smp_write(struct file *file,
> > - const char __user *user_buf,
> > - size_t count, loff_t *ppos)
> > -{
> > - struct hci_dev *hdev = file->private_data;
> > - bool enable;
> > - int err;
> > -
> > - err = kstrtobool_from_user(user_buf, count, &enable);
> > - if (err)
> > - return err;
> > -
> > if (enable == hci_dev_test_flag(hdev, HCI_FORCE_BREDR_SMP))
> > return -EALREADY;
> >
> > @@ -3399,16 +3376,9 @@ static ssize_t force_bredr_smp_write(struct file *file,
> >
> > hci_dev_change_flag(hdev, HCI_FORCE_BREDR_SMP);
> >
> > - return count;
> > + return 0;
> > }
> >
> > -static const struct file_operations force_bredr_smp_fops = {
> > - .open = simple_open,
> > - .read = force_bredr_smp_read,
> > - .write = force_bredr_smp_write,
> > - .llseek = default_llseek,
> > -};
> > -
> > int smp_register(struct hci_dev *hdev)
> > {
> > struct l2cap_chan *chan;
> > @@ -3433,17 +3403,7 @@ int smp_register(struct hci_dev *hdev)
> >
> > hdev->smp_data = chan;
> >
> > - /* If the controller does not support BR/EDR Secure Connections
> > - * feature, then the BR/EDR SMP channel shall not be present.
> > - *
> > - * To test this with Bluetooth 4.0 controllers, create a debugfs
> > - * switch that allows forcing BR/EDR SMP support and accepting
> > - * cross-transport pairing on non-AES encrypted connections.
> > - */
> > if (!lmp_sc_capable(hdev)) {
> > - debugfs_create_file("force_bredr_smp", 0644, hdev->debugfs,
> > - hdev, &force_bredr_smp_fops);
> > -
> > /* Flag can be already set here (due to power toggle) */
> > if (!hci_dev_test_flag(hdev, HCI_FORCE_BREDR_SMP))
> > return 0;
> > diff --git a/net/bluetooth/smp.h b/net/bluetooth/smp.h
> > index 121edadd5f8d..fc35a8bf358e 100644
> > --- a/net/bluetooth/smp.h
> > +++ b/net/bluetooth/smp.h
> > @@ -193,6 +193,8 @@ bool smp_irk_matches(struct hci_dev *hdev, const u8 irk[16],
> > int smp_generate_rpa(struct hci_dev *hdev, const u8 irk[16], bdaddr_t *rpa);
> > int smp_generate_oob(struct hci_dev *hdev, u8 hash[16], u8 rand[16]);
> >
> > +int smp_force_bredr(struct hci_dev *hdev, bool enable);
> > +
> > int smp_register(struct hci_dev *hdev);
> > void smp_unregister(struct hci_dev *hdev);
> >
> > --
> > 2.28.0.618.gf4bc123cb7-goog
> >
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] Bluetooth: Move force_bredr_smp debugfs into hci_debugfs_create_bredr
2020-10-27 15:49 ` Alain Michaud
@ 2020-10-27 17:17 ` Luiz Augusto von Dentz
0 siblings, 0 replies; 5+ messages in thread
From: Luiz Augusto von Dentz @ 2020-10-27 17:17 UTC (permalink / raw)
To: Alain Michaud
Cc: Claire Chang, Marcel Holtmann, Johan Hedberg, David S. Miller,
Jakub Kicinski, netdev, BlueZ, lkml
On Tue, Oct 27, 2020 at 10:12 AM Alain Michaud <alainmichaud@google.com> wrote:
>
> Friendly ping and adding my review-by tag.
>
>
> On Wed, Oct 7, 2020 at 12:38 AM Claire Chang <tientzu@chromium.org> wrote:
> >
> > Hi,
> >
> > This patch is to fix the kernel error
> > [ 46.271811] debugfs: File 'force_bredr_smp' in directory 'hci0'
> > already present!
> >
> > When powering off and on the bluetooth, the smp_register will try to create the
> > force_bredr_smp entry again.
> > Move the creation to hci_debugfs_create_bredr so the force_bredr_smp entry will
> > only be created when HCI_SETUP and HCI_CONFIG are not set.
> >
> > Thanks,
> > Claire
> >
> > On Tue, Sep 29, 2020 at 4:03 PM Claire Chang <tientzu@chromium.org> wrote:
> > >
> > > Avoid multiple attempts to create the debugfs entry, force_bredr_smp,
> > > by moving it from the SMP registration to the BR/EDR controller init
> > > section. hci_debugfs_create_bredr is only called when HCI_SETUP and
> > > HCI_CONFIG is not set.
> > >
> > > Signed-off-by: Claire Chang <tientzu@chromium.org>
> Reviewed-by: Alain Michaud <alainm@chromium.org>
Reviewed-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > > ---
> > > v2: correct a typo in commit message
> > >
> > > net/bluetooth/hci_debugfs.c | 50 +++++++++++++++++++++++++++++++++++++
> > > net/bluetooth/smp.c | 44 ++------------------------------
> > > net/bluetooth/smp.h | 2 ++
> > > 3 files changed, 54 insertions(+), 42 deletions(-)
> > >
> > > diff --git a/net/bluetooth/hci_debugfs.c b/net/bluetooth/hci_debugfs.c
> > > index 5e8af2658e44..4626e0289a97 100644
> > > --- a/net/bluetooth/hci_debugfs.c
> > > +++ b/net/bluetooth/hci_debugfs.c
> > > @@ -494,6 +494,45 @@ static int auto_accept_delay_get(void *data, u64 *val)
> > > DEFINE_SIMPLE_ATTRIBUTE(auto_accept_delay_fops, auto_accept_delay_get,
> > > auto_accept_delay_set, "%llu\n");
> > >
> > > +static ssize_t force_bredr_smp_read(struct file *file,
> > > + char __user *user_buf,
> > > + size_t count, loff_t *ppos)
> > > +{
> > > + struct hci_dev *hdev = file->private_data;
> > > + char buf[3];
> > > +
> > > + buf[0] = hci_dev_test_flag(hdev, HCI_FORCE_BREDR_SMP) ? 'Y' : 'N';
> > > + buf[1] = '\n';
> > > + buf[2] = '\0';
> > > + return simple_read_from_buffer(user_buf, count, ppos, buf, 2);
> > > +}
> > > +
> > > +static ssize_t force_bredr_smp_write(struct file *file,
> > > + const char __user *user_buf,
> > > + size_t count, loff_t *ppos)
> > > +{
> > > + struct hci_dev *hdev = file->private_data;
> > > + bool enable;
> > > + int err;
> > > +
> > > + err = kstrtobool_from_user(user_buf, count, &enable);
> > > + if (err)
> > > + return err;
> > > +
> > > + err = smp_force_bredr(hdev, enable);
> > > + if (err)
> > > + return err;
> > > +
> > > + return count;
> > > +}
> > > +
> > > +static const struct file_operations force_bredr_smp_fops = {
> > > + .open = simple_open,
> > > + .read = force_bredr_smp_read,
> > > + .write = force_bredr_smp_write,
> > > + .llseek = default_llseek,
> > > +};
> > > +
> > > static int idle_timeout_set(void *data, u64 val)
> > > {
> > > struct hci_dev *hdev = data;
> > > @@ -589,6 +628,17 @@ void hci_debugfs_create_bredr(struct hci_dev *hdev)
> > > debugfs_create_file("voice_setting", 0444, hdev->debugfs, hdev,
> > > &voice_setting_fops);
> > >
> > > + /* If the controller does not support BR/EDR Secure Connections
> > > + * feature, then the BR/EDR SMP channel shall not be present.
> > > + *
> > > + * To test this with Bluetooth 4.0 controllers, create a debugfs
> > > + * switch that allows forcing BR/EDR SMP support and accepting
> > > + * cross-transport pairing on non-AES encrypted connections.
> > > + */
> > > + if (!lmp_sc_capable(hdev))
> > > + debugfs_create_file("force_bredr_smp", 0644, hdev->debugfs,
> > > + hdev, &force_bredr_smp_fops);
> > > +
> > > if (lmp_ssp_capable(hdev)) {
> > > debugfs_create_file("ssp_debug_mode", 0444, hdev->debugfs,
> > > hdev, &ssp_debug_mode_fops);
> > > diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
> > > index 433227f96c73..8b817e4358fd 100644
> > > --- a/net/bluetooth/smp.c
> > > +++ b/net/bluetooth/smp.c
> > > @@ -3353,31 +3353,8 @@ static void smp_del_chan(struct l2cap_chan *chan)
> > > l2cap_chan_put(chan);
> > > }
> > >
> > > -static ssize_t force_bredr_smp_read(struct file *file,
> > > - char __user *user_buf,
> > > - size_t count, loff_t *ppos)
> > > +int smp_force_bredr(struct hci_dev *hdev, bool enable)
> > > {
> > > - struct hci_dev *hdev = file->private_data;
> > > - char buf[3];
> > > -
> > > - buf[0] = hci_dev_test_flag(hdev, HCI_FORCE_BREDR_SMP) ? 'Y': 'N';
> > > - buf[1] = '\n';
> > > - buf[2] = '\0';
> > > - return simple_read_from_buffer(user_buf, count, ppos, buf, 2);
> > > -}
> > > -
> > > -static ssize_t force_bredr_smp_write(struct file *file,
> > > - const char __user *user_buf,
> > > - size_t count, loff_t *ppos)
> > > -{
> > > - struct hci_dev *hdev = file->private_data;
> > > - bool enable;
> > > - int err;
> > > -
> > > - err = kstrtobool_from_user(user_buf, count, &enable);
> > > - if (err)
> > > - return err;
> > > -
> > > if (enable == hci_dev_test_flag(hdev, HCI_FORCE_BREDR_SMP))
> > > return -EALREADY;
> > >
> > > @@ -3399,16 +3376,9 @@ static ssize_t force_bredr_smp_write(struct file *file,
> > >
> > > hci_dev_change_flag(hdev, HCI_FORCE_BREDR_SMP);
> > >
> > > - return count;
> > > + return 0;
> > > }
> > >
> > > -static const struct file_operations force_bredr_smp_fops = {
> > > - .open = simple_open,
> > > - .read = force_bredr_smp_read,
> > > - .write = force_bredr_smp_write,
> > > - .llseek = default_llseek,
> > > -};
> > > -
> > > int smp_register(struct hci_dev *hdev)
> > > {
> > > struct l2cap_chan *chan;
> > > @@ -3433,17 +3403,7 @@ int smp_register(struct hci_dev *hdev)
> > >
> > > hdev->smp_data = chan;
> > >
> > > - /* If the controller does not support BR/EDR Secure Connections
> > > - * feature, then the BR/EDR SMP channel shall not be present.
> > > - *
> > > - * To test this with Bluetooth 4.0 controllers, create a debugfs
> > > - * switch that allows forcing BR/EDR SMP support and accepting
> > > - * cross-transport pairing on non-AES encrypted connections.
> > > - */
> > > if (!lmp_sc_capable(hdev)) {
> > > - debugfs_create_file("force_bredr_smp", 0644, hdev->debugfs,
> > > - hdev, &force_bredr_smp_fops);
> > > -
> > > /* Flag can be already set here (due to power toggle) */
> > > if (!hci_dev_test_flag(hdev, HCI_FORCE_BREDR_SMP))
> > > return 0;
> > > diff --git a/net/bluetooth/smp.h b/net/bluetooth/smp.h
> > > index 121edadd5f8d..fc35a8bf358e 100644
> > > --- a/net/bluetooth/smp.h
> > > +++ b/net/bluetooth/smp.h
> > > @@ -193,6 +193,8 @@ bool smp_irk_matches(struct hci_dev *hdev, const u8 irk[16],
> > > int smp_generate_rpa(struct hci_dev *hdev, const u8 irk[16], bdaddr_t *rpa);
> > > int smp_generate_oob(struct hci_dev *hdev, u8 hash[16], u8 rand[16]);
> > >
> > > +int smp_force_bredr(struct hci_dev *hdev, bool enable);
> > > +
> > > int smp_register(struct hci_dev *hdev);
> > > void smp_unregister(struct hci_dev *hdev);
> > >
> > > --
> > > 2.28.0.618.gf4bc123cb7-goog
> > >
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] Bluetooth: Move force_bredr_smp debugfs into hci_debugfs_create_bredr
2020-09-29 8:03 [PATCH v2] Bluetooth: Move force_bredr_smp debugfs into hci_debugfs_create_bredr Claire Chang
2020-10-07 4:38 ` Claire Chang
@ 2020-11-09 12:57 ` Marcel Holtmann
1 sibling, 0 replies; 5+ messages in thread
From: Marcel Holtmann @ 2020-11-09 12:57 UTC (permalink / raw)
To: Claire Chang
Cc: Johan Hedberg, David S. Miller, kuba, netdev, linux-bluetooth,
linux-kernel
Hi Claire,
> Avoid multiple attempts to create the debugfs entry, force_bredr_smp,
> by moving it from the SMP registration to the BR/EDR controller init
> section. hci_debugfs_create_bredr is only called when HCI_SETUP and
> HCI_CONFIG is not set.
>
> Signed-off-by: Claire Chang <tientzu@chromium.org>
> ---
> v2: correct a typo in commit message
>
> net/bluetooth/hci_debugfs.c | 50 +++++++++++++++++++++++++++++++++++++
> net/bluetooth/smp.c | 44 ++------------------------------
> net/bluetooth/smp.h | 2 ++
> 3 files changed, 54 insertions(+), 42 deletions(-)
patch has been applied to bluetooth-next tree.
Regards
Marcel
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-11-09 13:47 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-29 8:03 [PATCH v2] Bluetooth: Move force_bredr_smp debugfs into hci_debugfs_create_bredr Claire Chang
2020-10-07 4:38 ` Claire Chang
2020-10-27 15:49 ` Alain Michaud
2020-10-27 17:17 ` Luiz Augusto von Dentz
2020-11-09 12:57 ` Marcel Holtmann
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).