diff mbox series

[v2] Bluetooth: Move force_bredr_smp debugfs into hci_debugfs_create_bredr

Message ID 20200929080324.632523-1-tientzu@chromium.org (mailing list archive)
State New, archived
Headers show
Series [v2] Bluetooth: Move force_bredr_smp debugfs into hci_debugfs_create_bredr | expand

Commit Message

Claire Chang Sept. 29, 2020, 8:03 a.m. UTC
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(-)

Comments

Claire Chang Oct. 7, 2020, 4:38 a.m. UTC | #1
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
>
Alain Michaud Oct. 27, 2020, 3:49 p.m. UTC | #2
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
> >
Luiz Augusto von Dentz Oct. 27, 2020, 5:17 p.m. UTC | #3
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
> > >
Marcel Holtmann Nov. 9, 2020, 12:57 p.m. UTC | #4
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
diff mbox series

Patch

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);